Due to some hardware changes, SA8775P has set the NO_SNOOP attribute
in its TLP for all the PCIe controllers. NO_SNOOP attribute when set,
the requester is indicating that there no cache coherency issues exit
for the addressed memory on the host i.e., memory is not cached. But
in reality, requester cannot assume this unless there is a complete
control/visibility over the addressed memory on the host.
And worst case, if the memory is cached on the host, it may lead to
memory corruption issues. It should be noted that the caching of memory
on the host is not solely dependent on the NO_SNOOP attribute in TLP.
So to avoid the corruption, this patch overrides the NO_SNOOP attribute
by setting the PCIE_PARF_NO_SNOOP_OVERIDE register. This patch is not
needed for other upstream supported platforms since they do not set
NO_SNOOP attribute by default.
This series is to enable cache snooping logic in both RC and EP driver
and add the "dma-coherent" property in dtsi to support cache coherency
in SA8775P platform.
Dependency
----------
Depends on:
https://lore.kernel.org/all/[email protected]/
https://lore.kernel.org/all/[email protected]/ [1]
V4 -> V5:
- Updated commit message in both Patch1 and patch2
- change variable name from no_snoop_override to
enable_cache_snoop
- rebased patch2 on top of [1]
v3 -> v4:
- added new cfg(cfg_1_34_0) for SA8775P in both RC and EP driver.
- populated a flag in the data structures instead of doing
of_device_is_compatible() in both RC and EP patch.
- update commit mesaage and added reveiwed-by tag in commit message
in dtsi patch.
v2 -> v3:
- update commit message(8755 -> 8775).
v1 -> v2:
- update cover letter with explanation.
- define each of these bits and ORing at usage time rather than
directly writing value in register.
Mrinmay Sarkar (3):
PCI: qcom: Enable cache coherency for SA8775P RC
PCI: qcom-ep: Enable cache coherency for SA8775P EP
arm64: dts: qcom: sa8775p: Mark PCIe EP controller as cache coherent
arch/arm64/boot/dts/qcom/sa8775p.dtsi | 1 +
drivers/pci/controller/dwc/pcie-qcom-ep.c | 20 +++++++++++++++++---
drivers/pci/controller/dwc/pcie-qcom.c | 20 +++++++++++++++++++-
3 files changed, 37 insertions(+), 4 deletions(-)
--
2.40.1
Due to some hardware changes, SA8775P has set the NO_SNOOP attribute
in its TLP for all the PCIe controllers. NO_SNOOP attribute when set,
the requester is indicating that there no cache coherency issues exit
for the addressed memory on the host i.e., memory is not cached. But
in reality, requester cannot assume this unless there is a complete
control/visibility over the addressed memory on the host.
And worst case, if the memory is cached on the host, it may lead to
memory corruption issues. It should be noted that the caching of memory
on the host is not solely dependent on the NO_SNOOP attribute in TLP.
So to avoid the corruption, this patch overrides the NO_SNOOP attribute
by setting the PCIE_PARF_NO_SNOOP_OVERIDE register. This patch is not
needed for other upstream supported platforms since they do not set
NO_SNOOP attribute by default.
8775 has IP version 1.34.0 so intruduce a new cfg(cfg_1_34_0) for this
platform. Assign enable_cache_snoop flag into struct qcom_pcie_cfg and
set it true in cfg_1_34_0 and enable cache snooping if this particular
flag is true.
Signed-off-by: Mrinmay Sarkar <[email protected]>
---
drivers/pci/controller/dwc/pcie-qcom.c | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 2ce2a3bd932b..872be7f7d7b3 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -51,6 +51,7 @@
#define PARF_SID_OFFSET 0x234
#define PARF_BDF_TRANSLATE_CFG 0x24c
#define PARF_SLV_ADDR_SPACE_SIZE 0x358
+#define PARF_NO_SNOOP_OVERIDE 0x3d4
#define PARF_DEVICE_TYPE 0x1000
#define PARF_BDF_TO_SID_TABLE_N 0x2000
@@ -117,6 +118,10 @@
/* PARF_LTSSM register fields */
#define LTSSM_EN BIT(8)
+/* PARF_NO_SNOOP_OVERIDE register fields */
+#define WR_NO_SNOOP_OVERIDE_EN BIT(1)
+#define RD_NO_SNOOP_OVERIDE_EN BIT(3)
+
/* PARF_DEVICE_TYPE register fields */
#define DEVICE_TYPE_RC 0x4
@@ -229,6 +234,7 @@ struct qcom_pcie_ops {
struct qcom_pcie_cfg {
const struct qcom_pcie_ops *ops;
+ bool enable_cache_snoop;
};
struct qcom_pcie {
@@ -961,6 +967,13 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
static int qcom_pcie_post_init_2_7_0(struct qcom_pcie *pcie)
{
+ const struct qcom_pcie_cfg *pcie_cfg = pcie->cfg;
+
+ /* Enable cache snooping for SA8775P */
+ if (pcie_cfg->enable_cache_snoop)
+ writel(WR_NO_SNOOP_OVERIDE_EN | RD_NO_SNOOP_OVERIDE_EN,
+ pcie->parf + PARF_NO_SNOOP_OVERIDE);
+
qcom_pcie_clear_hpc(pcie->pci);
return 0;
@@ -1334,6 +1347,11 @@ static const struct qcom_pcie_cfg cfg_1_9_0 = {
.ops = &ops_1_9_0,
};
+static const struct qcom_pcie_cfg cfg_1_34_0 = {
+ .ops = &ops_1_9_0,
+ .enable_cache_snoop = true,
+};
+
static const struct qcom_pcie_cfg cfg_2_1_0 = {
.ops = &ops_2_1_0,
};
@@ -1630,7 +1648,7 @@ static const struct of_device_id qcom_pcie_match[] = {
{ .compatible = "qcom,pcie-msm8996", .data = &cfg_2_3_2 },
{ .compatible = "qcom,pcie-qcs404", .data = &cfg_2_4_0 },
{ .compatible = "qcom,pcie-sa8540p", .data = &cfg_1_9_0 },
- { .compatible = "qcom,pcie-sa8775p", .data = &cfg_1_9_0},
+ { .compatible = "qcom,pcie-sa8775p", .data = &cfg_1_34_0},
{ .compatible = "qcom,pcie-sc7280", .data = &cfg_1_9_0 },
{ .compatible = "qcom,pcie-sc8180x", .data = &cfg_1_9_0 },
{ .compatible = "qcom,pcie-sc8280xp", .data = &cfg_1_9_0 },
--
2.40.1
Due to some hardware changes, SA8775P has set the NO_SNOOP attribute
in its TLP for all the PCIe controllers. NO_SNOOP attribute when set,
the requester is indicating that there no cache coherency issues exit
for the addressed memory on the host i.e., memory is not cached. But
in reality, requester cannot assume this unless there is a complete
control/visibility over the addressed memory on the host.
And worst case, if the memory is cached on the host, it may lead to
memory corruption issues. It should be noted that the caching of memory
on the host is not solely dependent on the NO_SNOOP attribute in TLP.
So to avoid the corruption, this patch overrides the NO_SNOOP attribute
by setting the PCIE_PARF_NO_SNOOP_OVERIDE register. This patch is not
needed for other upstream supported platforms since they do not set
NO_SNOOP attribute by default.
Signed-off-by: Mrinmay Sarkar <[email protected]>
---
drivers/pci/controller/dwc/pcie-qcom-ep.c | 20 +++++++++++++++++---
1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
index 89d06a3e6e06..369954649254 100644
--- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
+++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
@@ -45,6 +45,7 @@
#define PARF_SLV_ADDR_MSB_CTRL 0x2c0
#define PARF_DBI_BASE_ADDR 0x350
#define PARF_DBI_BASE_ADDR_HI 0x354
+#define PARF_NO_SNOOP_OVERIDE 0x3d4
#define PARF_SLV_ADDR_SPACE_SIZE 0x358
#define PARF_SLV_ADDR_SPACE_SIZE_HI 0x35c
#define PARF_ATU_BASE_ADDR 0x634
@@ -86,6 +87,10 @@
#define PARF_DEBUG_INT_CFG_BUS_MASTER_EN BIT(2)
#define PARF_DEBUG_INT_RADM_PM_TURNOFF BIT(3)
+/* PARF_NO_SNOOP_OVERIDE register fields */
+#define WR_NO_SNOOP_OVERIDE_EN BIT(1)
+#define RD_NO_SNOOP_OVERIDE_EN BIT(3)
+
/* PARF_DEVICE_TYPE register fields */
#define PARF_DEVICE_TYPE_EP 0x0
@@ -152,9 +157,11 @@ enum qcom_pcie_ep_link_status {
/**
* struct qcom_pcie_ep_cfg - Per SoC config struct
* @hdma_support: HDMA support on this SoC
+ * @enable_cache_snoop: enable cache snooping on this SoC
*/
struct qcom_pcie_ep_cfg {
bool hdma_support;
+ bool enable_cache_snoop;
};
/**
@@ -175,6 +182,7 @@ struct qcom_pcie_ep_cfg {
* @num_clks: PCIe clocks count
* @perst_en: Flag for PERST enable
* @perst_sep_en: Flag for PERST separation enable
+ * @cfg: PCIe EP config struct
* @link_status: PCIe Link status
* @global_irq: Qualcomm PCIe specific Global IRQ
* @perst_irq: PERST# IRQ
@@ -202,6 +210,7 @@ struct qcom_pcie_ep {
u32 perst_en;
u32 perst_sep_en;
+ const struct qcom_pcie_ep_cfg *cfg;
enum qcom_pcie_ep_link_status link_status;
int global_irq;
int perst_irq;
@@ -497,6 +506,11 @@ static int qcom_pcie_perst_deassert(struct dw_pcie *pci)
val |= BIT(8);
writel_relaxed(val, pcie_ep->parf + PARF_LTSSM);
+ /* Enable cache snooping for SA8775P */
+ if (pcie_ep->cfg && pcie_ep->cfg->enable_cache_snoop)
+ writel_relaxed(WR_NO_SNOOP_OVERIDE_EN | RD_NO_SNOOP_OVERIDE_EN,
+ pcie_ep->parf + PARF_NO_SNOOP_OVERIDE);
+
return 0;
err_disable_resources:
@@ -811,7 +825,6 @@ static const struct dw_pcie_ep_ops pci_ep_ops = {
static int qcom_pcie_ep_probe(struct platform_device *pdev)
{
- const struct qcom_pcie_ep_cfg *cfg;
struct device *dev = &pdev->dev;
struct qcom_pcie_ep *pcie_ep;
char *name;
@@ -826,8 +839,8 @@ static int qcom_pcie_ep_probe(struct platform_device *pdev)
pcie_ep->pci.ep.ops = &pci_ep_ops;
pcie_ep->pci.edma.nr_irqs = 1;
- cfg = of_device_get_match_data(dev);
- if (cfg && cfg->hdma_support) {
+ pcie_ep->cfg = of_device_get_match_data(dev);
+ if (pcie_ep->cfg && pcie_ep->cfg->hdma_support) {
pcie_ep->pci.edma.ll_wr_cnt = 8;
pcie_ep->pci.edma.ll_rd_cnt = 8;
pcie_ep->pci.edma.mf = EDMA_MF_HDMA_NATIVE;
@@ -893,6 +906,7 @@ static void qcom_pcie_ep_remove(struct platform_device *pdev)
static const struct qcom_pcie_ep_cfg cfg_1_34_0 = {
.hdma_support = true,
+ .enable_cache_snoop = true,
};
static const struct of_device_id qcom_pcie_ep_match[] = {
--
2.40.1
The PCIe EP controller on SA8775P supports cache coherency, hence add
the "dma-coherent" property to mark it as such.
Signed-off-by: Mrinmay Sarkar <[email protected]>
Reviewed-by: Manivannan Sadhasivam <[email protected]>
---
arch/arm64/boot/dts/qcom/sa8775p.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/boot/dts/qcom/sa8775p.dtsi b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
index d98020273f0e..53c31c7034a8 100644
--- a/arch/arm64/boot/dts/qcom/sa8775p.dtsi
+++ b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
@@ -3713,6 +3713,7 @@ pcie0_ep: pcie-ep@1c00000 {
<&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_PCIE_0 0>;
interconnect-names = "pcie-mem", "cpu-pcie";
+ dma-coherent;
iommus = <&pcie_smmu 0x0000 0x7f>;
resets = <&gcc GCC_PCIE_0_BCR>;
reset-names = "core";
--
2.40.1
On Fri, Feb 23, 2024 at 07:33:38PM +0530, Mrinmay Sarkar wrote:
> Due to some hardware changes, SA8775P has set the NO_SNOOP attribute
> in its TLP for all the PCIe controllers. NO_SNOOP attribute when set,
> the requester is indicating that there no cache coherency issues exit
> for the addressed memory on the host i.e., memory is not cached. But
> in reality, requester cannot assume this unless there is a complete
> control/visibility over the addressed memory on the host.
s/that there no/that no/
s/exit/exist/
Forgive my ignorance here. It sounds like the cache coherency issue
would refer to system memory, so the relevant No Snoop attribute would
be in DMA transactions, i.e., Memory Reads or Writes initiated by PCIe
Endpoints. But it looks like this patch would affect TLPs initiated
by the Root Complex, not those from Endpoints, so I'm confused about
how this works.
If this were in the qcom-ep driver, it would make sense that setting
No Snoop in the TLPs initiated by the Endpoint could be a problem, but
that doesn't seem to be what this patch is concerned with.
> And worst case, if the memory is cached on the host, it may lead to
> memory corruption issues. It should be noted that the caching of memory
> on the host is not solely dependent on the NO_SNOOP attribute in TLP.
>
> So to avoid the corruption, this patch overrides the NO_SNOOP attribute
> by setting the PCIE_PARF_NO_SNOOP_OVERIDE register. This patch is not
> needed for other upstream supported platforms since they do not set
> NO_SNOOP attribute by default.
>
> 8775 has IP version 1.34.0 so intruduce a new cfg(cfg_1_34_0) for this
> platform. Assign enable_cache_snoop flag into struct qcom_pcie_cfg and
> set it true in cfg_1_34_0 and enable cache snooping if this particular
> flag is true.
s/intruduce/introduce/
Bjorn
On 23.02.2024 15:03, Mrinmay Sarkar wrote:
> Due to some hardware changes, SA8775P has set the NO_SNOOP attribute
> in its TLP for all the PCIe controllers. NO_SNOOP attribute when set,
> the requester is indicating that there no cache coherency issues exit
> for the addressed memory on the host i.e., memory is not cached. But
> in reality, requester cannot assume this unless there is a complete
> control/visibility over the addressed memory on the host.
>
> And worst case, if the memory is cached on the host, it may lead to
> memory corruption issues. It should be noted that the caching of memory
> on the host is not solely dependent on the NO_SNOOP attribute in TLP.
>
> So to avoid the corruption, this patch overrides the NO_SNOOP attribute
> by setting the PCIE_PARF_NO_SNOOP_OVERIDE register. This patch is not
> needed for other upstream supported platforms since they do not set
> NO_SNOOP attribute by default.
>
> Signed-off-by: Mrinmay Sarkar <[email protected]>
> ---
> drivers/pci/controller/dwc/pcie-qcom-ep.c | 20 +++++++++++++++++---
> 1 file changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> index 89d06a3e6e06..369954649254 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> @@ -45,6 +45,7 @@
> #define PARF_SLV_ADDR_MSB_CTRL 0x2c0
> #define PARF_DBI_BASE_ADDR 0x350
> #define PARF_DBI_BASE_ADDR_HI 0x354
> +#define PARF_NO_SNOOP_OVERIDE 0x3d4
Any reason for this to be unsorted?
Konrad
On 23/02/2024 15:03, Mrinmay Sarkar wrote:
> Due to some hardware changes, SA8775P has set the NO_SNOOP attribute
> in its TLP for all the PCIe controllers. NO_SNOOP attribute when set,
> the requester is indicating that there no cache coherency issues exit
> for the addressed memory on the host i.e., memory is not cached. But
> in reality, requester cannot assume this unless there is a complete
> control/visibility over the addressed memory on the host.
>
> And worst case, if the memory is cached on the host, it may lead to
> memory corruption issues. It should be noted that the caching of memory
> on the host is not solely dependent on the NO_SNOOP attribute in TLP.
>
> So to avoid the corruption, this patch overrides the NO_SNOOP attribute
> by setting the PCIE_PARF_NO_SNOOP_OVERIDE register. This patch is not
> needed for other upstream supported platforms since they do not set
> NO_SNOOP attribute by default.
>
> This series is to enable cache snooping logic in both RC and EP driver
> and add the "dma-coherent" property in dtsi to support cache coherency
> in SA8775P platform.
Please confirm that your patchset passes 100% dtbs_check.
Best regards,
Krzysztof
On 2/24/2024 4:24 AM, Bjorn Helgaas wrote:
> On Fri, Feb 23, 2024 at 07:33:38PM +0530, Mrinmay Sarkar wrote:
>> Due to some hardware changes, SA8775P has set the NO_SNOOP attribute
>> in its TLP for all the PCIe controllers. NO_SNOOP attribute when set,
>> the requester is indicating that there no cache coherency issues exit
>> for the addressed memory on the host i.e., memory is not cached. But
>> in reality, requester cannot assume this unless there is a complete
>> control/visibility over the addressed memory on the host.
> s/that there no/that no/
> s/exit/exist/
>
> Forgive my ignorance here. It sounds like the cache coherency issue
> would refer to system memory, so the relevant No Snoop attribute would
> be in DMA transactions, i.e., Memory Reads or Writes initiated by PCIe
> Endpoints. But it looks like this patch would affect TLPs initiated
> by the Root Complex, not those from Endpoints, so I'm confused about
> how this works.
>
> If this were in the qcom-ep driver, it would make sense that setting
> No Snoop in the TLPs initiated by the Endpoint could be a problem, but
> that doesn't seem to be what this patch is concerned with.
I think in multiprocessor system cache coherency issue might occur.
and RC as well needs to snoop cache to avoid coherency as it is not
enable by default.
and we are enabling this feature for qcom-ep driver as well.
it is in patch2.
Thanks
Mrinmay
>> And worst case, if the memory is cached on the host, it may lead to
>> memory corruption issues. It should be noted that the caching of memory
>> on the host is not solely dependent on the NO_SNOOP attribute in TLP.
>>
>> So to avoid the corruption, this patch overrides the NO_SNOOP attribute
>> by setting the PCIE_PARF_NO_SNOOP_OVERIDE register. This patch is not
>> needed for other upstream supported platforms since they do not set
>> NO_SNOOP attribute by default.
>>
>> 8775 has IP version 1.34.0 so intruduce a new cfg(cfg_1_34_0) for this
>> platform. Assign enable_cache_snoop flag into struct qcom_pcie_cfg and
>> set it true in cfg_1_34_0 and enable cache snooping if this particular
>> flag is true.
> s/intruduce/introduce/
>
> Bjorn
On 2/24/2024 5:37 AM, Konrad Dybcio wrote:
> On 23.02.2024 15:03, Mrinmay Sarkar wrote:
>> Due to some hardware changes, SA8775P has set the NO_SNOOP attribute
>> in its TLP for all the PCIe controllers. NO_SNOOP attribute when set,
>> the requester is indicating that there no cache coherency issues exit
>> for the addressed memory on the host i.e., memory is not cached. But
>> in reality, requester cannot assume this unless there is a complete
>> control/visibility over the addressed memory on the host.
>>
>> And worst case, if the memory is cached on the host, it may lead to
>> memory corruption issues. It should be noted that the caching of memory
>> on the host is not solely dependent on the NO_SNOOP attribute in TLP.
>>
>> So to avoid the corruption, this patch overrides the NO_SNOOP attribute
>> by setting the PCIE_PARF_NO_SNOOP_OVERIDE register. This patch is not
>> needed for other upstream supported platforms since they do not set
>> NO_SNOOP attribute by default.
>>
>> Signed-off-by: Mrinmay Sarkar <[email protected]>
>> ---
>> drivers/pci/controller/dwc/pcie-qcom-ep.c | 20 +++++++++++++++++---
>> 1 file changed, 17 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
>> index 89d06a3e6e06..369954649254 100644
>> --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
>> +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
>> @@ -45,6 +45,7 @@
>> #define PARF_SLV_ADDR_MSB_CTRL 0x2c0
>> #define PARF_DBI_BASE_ADDR 0x350
>> #define PARF_DBI_BASE_ADDR_HI 0x354
>> +#define PARF_NO_SNOOP_OVERIDE 0x3d4
> Any reason for this to be unsorted?
>
> Konrad
Yes, this should be sorted. Will fix this in next series.
Thanks
Mrinmay
On 2/24/2024 3:49 PM, Krzysztof Kozlowski wrote:
> On 23/02/2024 15:03, Mrinmay Sarkar wrote:
>> Due to some hardware changes, SA8775P has set the NO_SNOOP attribute
>> in its TLP for all the PCIe controllers. NO_SNOOP attribute when set,
>> the requester is indicating that there no cache coherency issues exit
>> for the addressed memory on the host i.e., memory is not cached. But
>> in reality, requester cannot assume this unless there is a complete
>> control/visibility over the addressed memory on the host.
>>
>> And worst case, if the memory is cached on the host, it may lead to
>> memory corruption issues. It should be noted that the caching of memory
>> on the host is not solely dependent on the NO_SNOOP attribute in TLP.
>>
>> So to avoid the corruption, this patch overrides the NO_SNOOP attribute
>> by setting the PCIE_PARF_NO_SNOOP_OVERIDE register. This patch is not
>> needed for other upstream supported platforms since they do not set
>> NO_SNOOP attribute by default.
>>
>> This series is to enable cache snooping logic in both RC and EP driver
>> and add the "dma-coherent" property in dtsi to support cache coherency
>> in SA8775P platform.
> Please confirm that your patchset passes 100% dtbs_check.
>
> Best regards,
> Krzysztof
I have run dtbs_check and it is passing.
Thanks
Mrinmay
>
On 28/02/2024 14:07, Mrinmay Sarkar wrote:
>
> On 2/24/2024 3:49 PM, Krzysztof Kozlowski wrote:
>> On 23/02/2024 15:03, Mrinmay Sarkar wrote:
>>> Due to some hardware changes, SA8775P has set the NO_SNOOP attribute
>>> in its TLP for all the PCIe controllers. NO_SNOOP attribute when set,
>>> the requester is indicating that there no cache coherency issues exit
>>> for the addressed memory on the host i.e., memory is not cached. But
>>> in reality, requester cannot assume this unless there is a complete
>>> control/visibility over the addressed memory on the host.
>>>
>>> And worst case, if the memory is cached on the host, it may lead to
>>> memory corruption issues. It should be noted that the caching of memory
>>> on the host is not solely dependent on the NO_SNOOP attribute in TLP.
>>>
>>> So to avoid the corruption, this patch overrides the NO_SNOOP attribute
>>> by setting the PCIE_PARF_NO_SNOOP_OVERIDE register. This patch is not
>>> needed for other upstream supported platforms since they do not set
>>> NO_SNOOP attribute by default.
>>>
>>> This series is to enable cache snooping logic in both RC and EP driver
>>> and add the "dma-coherent" property in dtsi to support cache coherency
>>> in SA8775P platform.
>> Please confirm that your patchset passes 100% dtbs_check.
>>
>> Best regards,
>> Krzysztof
>
> I have run dtbs_check and it is passing.
Hm, last time I checked dma-coherent was not allowed.
Best regards,
Krzysztof
On Wed, Feb 28, 2024 at 06:34:11PM +0530, Mrinmay Sarkar wrote:
> On 2/24/2024 4:24 AM, Bjorn Helgaas wrote:
> > On Fri, Feb 23, 2024 at 07:33:38PM +0530, Mrinmay Sarkar wrote:
> > > Due to some hardware changes, SA8775P has set the NO_SNOOP attribute
> > > in its TLP for all the PCIe controllers. NO_SNOOP attribute when set,
> > > the requester is indicating that there no cache coherency issues exit
> > > for the addressed memory on the host i.e., memory is not cached. But
> > > in reality, requester cannot assume this unless there is a complete
> > > control/visibility over the addressed memory on the host.
> >
> > Forgive my ignorance here. It sounds like the cache coherency issue
> > would refer to system memory, so the relevant No Snoop attribute would
> > be in DMA transactions, i.e., Memory Reads or Writes initiated by PCIe
> > Endpoints. But it looks like this patch would affect TLPs initiated
> > by the Root Complex, not those from Endpoints, so I'm confused about
> > how this works.
> >
> > If this were in the qcom-ep driver, it would make sense that setting
> > No Snoop in the TLPs initiated by the Endpoint could be a problem, but
> > that doesn't seem to be what this patch is concerned with.
>
> I think in multiprocessor system cache coherency issue might occur.
> and RC as well needs to snoop cache to avoid coherency as it is not
> enable by default.
My mental picture isn't detailed enough, so I'm still confused. We're
talking about TLPs initiated by the RC. Normally these would be
because a driver did a CPU load or store to a PCIe device MMIO space,
not to system memory.
But I guess you're suggesting the RC can initiate a TLP with a system
memory address? And this TLP would be routed not to a Root Port or to
downstream devices, but it would instead be kind of a loopback and be
routed back up through the RC and maybe IOMMU, to system memory?
I would have expected accesses like this to be routed directly to
system memory without ever reaching the PCIe RC.
> and we are enabling this feature for qcom-ep driver as well.
> it is in patch2.
>
> Thanks
> Mrinmay
>
> > > And worst case, if the memory is cached on the host, it may lead to
> > > memory corruption issues. It should be noted that the caching of memory
> > > on the host is not solely dependent on the NO_SNOOP attribute in TLP.
> > >
> > > So to avoid the corruption, this patch overrides the NO_SNOOP attribute
> > > by setting the PCIE_PARF_NO_SNOOP_OVERIDE register. This patch is not
> > > needed for other upstream supported platforms since they do not set
> > > NO_SNOOP attribute by default.
> > >
> > > 8775 has IP version 1.34.0 so intruduce a new cfg(cfg_1_34_0) for this
> > > platform. Assign enable_cache_snoop flag into struct qcom_pcie_cfg and
> > > set it true in cfg_1_34_0 and enable cache snooping if this particular
> > > flag is true.
> > s/intruduce/introduce/
> >
> > Bjorn
On Wed, Feb 28, 2024 at 09:02:11AM -0600, Bjorn Helgaas wrote:
> On Wed, Feb 28, 2024 at 06:34:11PM +0530, Mrinmay Sarkar wrote:
> > On 2/24/2024 4:24 AM, Bjorn Helgaas wrote:
> > > On Fri, Feb 23, 2024 at 07:33:38PM +0530, Mrinmay Sarkar wrote:
> > > > Due to some hardware changes, SA8775P has set the NO_SNOOP attribute
> > > > in its TLP for all the PCIe controllers. NO_SNOOP attribute when set,
> > > > the requester is indicating that there no cache coherency issues exit
> > > > for the addressed memory on the host i.e., memory is not cached. But
> > > > in reality, requester cannot assume this unless there is a complete
> > > > control/visibility over the addressed memory on the host.
> > >
> > > Forgive my ignorance here. It sounds like the cache coherency issue
> > > would refer to system memory, so the relevant No Snoop attribute would
> > > be in DMA transactions, i.e., Memory Reads or Writes initiated by PCIe
> > > Endpoints. But it looks like this patch would affect TLPs initiated
> > > by the Root Complex, not those from Endpoints, so I'm confused about
> > > how this works.
> > >
> > > If this were in the qcom-ep driver, it would make sense that setting
> > > No Snoop in the TLPs initiated by the Endpoint could be a problem, but
> > > that doesn't seem to be what this patch is concerned with.
> >
> > I think in multiprocessor system cache coherency issue might occur.
> > and RC as well needs to snoop cache to avoid coherency as it is not
> > enable by default.
>
> My mental picture isn't detailed enough, so I'm still confused. We're
> talking about TLPs initiated by the RC. Normally these would be
> because a driver did a CPU load or store to a PCIe device MMIO space,
> not to system memory.
>
Endpoint can expose its system memory as a BAR to the host. In that case, the
cache coherency issue would apply for TLPs originating from RC as well.
- Mani
> But I guess you're suggesting the RC can initiate a TLP with a system
> memory address? And this TLP would be routed not to a Root Port or to
> downstream devices, but it would instead be kind of a loopback and be
> routed back up through the RC and maybe IOMMU, to system memory?
>
> I would have expected accesses like this to be routed directly to
> system memory without ever reaching the PCIe RC.
>
> > and we are enabling this feature for qcom-ep driver as well.
> > it is in patch2.
> >
> > Thanks
> > Mrinmay
> >
> > > > And worst case, if the memory is cached on the host, it may lead to
> > > > memory corruption issues. It should be noted that the caching of memory
> > > > on the host is not solely dependent on the NO_SNOOP attribute in TLP.
> > > >
> > > > So to avoid the corruption, this patch overrides the NO_SNOOP attribute
> > > > by setting the PCIE_PARF_NO_SNOOP_OVERIDE register. This patch is not
> > > > needed for other upstream supported platforms since they do not set
> > > > NO_SNOOP attribute by default.
> > > >
> > > > 8775 has IP version 1.34.0 so intruduce a new cfg(cfg_1_34_0) for this
> > > > platform. Assign enable_cache_snoop flag into struct qcom_pcie_cfg and
> > > > set it true in cfg_1_34_0 and enable cache snooping if this particular
> > > > flag is true.
> > > s/intruduce/introduce/
> > >
> > > Bjorn
--
மணிவண்ணன் சதாசிவம்
On Wed, Feb 28, 2024 at 10:44:12PM +0530, Manivannan Sadhasivam wrote:
> On Wed, Feb 28, 2024 at 09:02:11AM -0600, Bjorn Helgaas wrote:
> > On Wed, Feb 28, 2024 at 06:34:11PM +0530, Mrinmay Sarkar wrote:
> > > On 2/24/2024 4:24 AM, Bjorn Helgaas wrote:
> > > > On Fri, Feb 23, 2024 at 07:33:38PM +0530, Mrinmay Sarkar wrote:
> > > > > Due to some hardware changes, SA8775P has set the NO_SNOOP attribute
> > > > > in its TLP for all the PCIe controllers. NO_SNOOP attribute when set,
> > > > > the requester is indicating that there no cache coherency issues exit
> > > > > for the addressed memory on the host i.e., memory is not cached. But
> > > > > in reality, requester cannot assume this unless there is a complete
> > > > > control/visibility over the addressed memory on the host.
> > > >
> > > > Forgive my ignorance here. It sounds like the cache coherency issue
> > > > would refer to system memory, so the relevant No Snoop attribute would
> > > > be in DMA transactions, i.e., Memory Reads or Writes initiated by PCIe
> > > > Endpoints. But it looks like this patch would affect TLPs initiated
> > > > by the Root Complex, not those from Endpoints, so I'm confused about
> > > > how this works.
> > > >
> > > > If this were in the qcom-ep driver, it would make sense that setting
> > > > No Snoop in the TLPs initiated by the Endpoint could be a problem, but
> > > > that doesn't seem to be what this patch is concerned with.
> > >
> > > I think in multiprocessor system cache coherency issue might occur.
> > > and RC as well needs to snoop cache to avoid coherency as it is not
> > > enable by default.
> >
> > My mental picture isn't detailed enough, so I'm still confused. We're
> > talking about TLPs initiated by the RC. Normally these would be
> > because a driver did a CPU load or store to a PCIe device MMIO space,
> > not to system memory.
>
> Endpoint can expose its system memory as a BAR to the host. In that case, the
> cache coherency issue would apply for TLPs originating from RC as well.
What PCIe transactions are involved here? So far I know about:
RC initiates Memory Read Request (or Write) with NO_SNOOP==0
...
EP responds with Completion with Data (for Read)
But I guess you're saying the EP would initiate other transactions in
the middle related to snooping? I don't know what those are.
> > But I guess you're suggesting the RC can initiate a TLP with a system
> > memory address? And this TLP would be routed not to a Root Port or to
> > downstream devices, but it would instead be kind of a loopback and be
> > routed back up through the RC and maybe IOMMU, to system memory?
> >
> > I would have expected accesses like this to be routed directly to
> > system memory without ever reaching the PCIe RC.
> >
> > > and we are enabling this feature for qcom-ep driver as well.
> > > it is in patch2.
> > >
> > > Thanks
> > > Mrinmay
> > >
> > > > > And worst case, if the memory is cached on the host, it may lead to
> > > > > memory corruption issues. It should be noted that the caching of memory
> > > > > on the host is not solely dependent on the NO_SNOOP attribute in TLP.
> > > > >
> > > > > So to avoid the corruption, this patch overrides the NO_SNOOP attribute
> > > > > by setting the PCIE_PARF_NO_SNOOP_OVERIDE register. This patch is not
> > > > > needed for other upstream supported platforms since they do not set
> > > > > NO_SNOOP attribute by default.
> > > > >
> > > > > 8775 has IP version 1.34.0 so intruduce a new cfg(cfg_1_34_0) for this
> > > > > platform. Assign enable_cache_snoop flag into struct qcom_pcie_cfg and
> > > > > set it true in cfg_1_34_0 and enable cache snooping if this particular
> > > > > flag is true.
> > > > s/intruduce/introduce/
> > > >
> > > > Bjorn
>
> --
> மணிவண்ணன் சதாசிவம்
On Wed, Feb 28, 2024 at 11:39:07AM -0600, Bjorn Helgaas wrote:
> On Wed, Feb 28, 2024 at 10:44:12PM +0530, Manivannan Sadhasivam wrote:
> > On Wed, Feb 28, 2024 at 09:02:11AM -0600, Bjorn Helgaas wrote:
> > > On Wed, Feb 28, 2024 at 06:34:11PM +0530, Mrinmay Sarkar wrote:
> > > > On 2/24/2024 4:24 AM, Bjorn Helgaas wrote:
> > > > > On Fri, Feb 23, 2024 at 07:33:38PM +0530, Mrinmay Sarkar wrote:
> > > > > > Due to some hardware changes, SA8775P has set the NO_SNOOP attribute
> > > > > > in its TLP for all the PCIe controllers. NO_SNOOP attribute when set,
> > > > > > the requester is indicating that there no cache coherency issues exit
> > > > > > for the addressed memory on the host i.e., memory is not cached. But
> > > > > > in reality, requester cannot assume this unless there is a complete
> > > > > > control/visibility over the addressed memory on the host.
> > > > >
> > > > > Forgive my ignorance here. It sounds like the cache coherency issue
> > > > > would refer to system memory, so the relevant No Snoop attribute would
> > > > > be in DMA transactions, i.e., Memory Reads or Writes initiated by PCIe
> > > > > Endpoints. But it looks like this patch would affect TLPs initiated
> > > > > by the Root Complex, not those from Endpoints, so I'm confused about
> > > > > how this works.
> > > > >
> > > > > If this were in the qcom-ep driver, it would make sense that setting
> > > > > No Snoop in the TLPs initiated by the Endpoint could be a problem, but
> > > > > that doesn't seem to be what this patch is concerned with.
> > > >
> > > > I think in multiprocessor system cache coherency issue might occur.
> > > > and RC as well needs to snoop cache to avoid coherency as it is not
> > > > enable by default.
> > >
> > > My mental picture isn't detailed enough, so I'm still confused. We're
> > > talking about TLPs initiated by the RC. Normally these would be
> > > because a driver did a CPU load or store to a PCIe device MMIO space,
> > > not to system memory.
> >
> > Endpoint can expose its system memory as a BAR to the host. In that case, the
> > cache coherency issue would apply for TLPs originating from RC as well.
>
> What PCIe transactions are involved here? So far I know about:
>
> RC initiates Memory Read Request (or Write) with NO_SNOOP==0
> ...
> EP responds with Completion with Data (for Read)
>
The memory on the endpoint may be cached (due to linear map and such). So if the
RC is initiating the MWd TLP with NO_SNOOP=1, then there would be coherency
issues because there is no guarantee that the memory is not cached on the
endpoint. So, not snooping the caches and directly writing to the DDR would
cause coherency issues on the endpoint as well.
- Mani
> But I guess you're saying the EP would initiate other transactions in
> the middle related to snooping? I don't know what those are.
>
> > > But I guess you're suggesting the RC can initiate a TLP with a system
> > > memory address? And this TLP would be routed not to a Root Port or to
> > > downstream devices, but it would instead be kind of a loopback and be
> > > routed back up through the RC and maybe IOMMU, to system memory?
> > >
> > > I would have expected accesses like this to be routed directly to
> > > system memory without ever reaching the PCIe RC.
> > >
> > > > and we are enabling this feature for qcom-ep driver as well.
> > > > it is in patch2.
> > > >
> > > > Thanks
> > > > Mrinmay
> > > >
> > > > > > And worst case, if the memory is cached on the host, it may lead to
> > > > > > memory corruption issues. It should be noted that the caching of memory
> > > > > > on the host is not solely dependent on the NO_SNOOP attribute in TLP.
> > > > > >
> > > > > > So to avoid the corruption, this patch overrides the NO_SNOOP attribute
> > > > > > by setting the PCIE_PARF_NO_SNOOP_OVERIDE register. This patch is not
> > > > > > needed for other upstream supported platforms since they do not set
> > > > > > NO_SNOOP attribute by default.
> > > > > >
> > > > > > 8775 has IP version 1.34.0 so intruduce a new cfg(cfg_1_34_0) for this
> > > > > > platform. Assign enable_cache_snoop flag into struct qcom_pcie_cfg and
> > > > > > set it true in cfg_1_34_0 and enable cache snooping if this particular
> > > > > > flag is true.
> > > > > s/intruduce/introduce/
> > > > >
> > > > > Bjorn
> >
> > --
> > மணிவண்ணன் சதாசிவம்
--
மணிவண்ணன் சதாசிவம்
On Thu, Feb 29, 2024 at 12:15:02AM +0530, Manivannan Sadhasivam wrote:
> On Wed, Feb 28, 2024 at 11:39:07AM -0600, Bjorn Helgaas wrote:
> > On Wed, Feb 28, 2024 at 10:44:12PM +0530, Manivannan Sadhasivam wrote:
> > > On Wed, Feb 28, 2024 at 09:02:11AM -0600, Bjorn Helgaas wrote:
> > > > On Wed, Feb 28, 2024 at 06:34:11PM +0530, Mrinmay Sarkar wrote:
> > > > > On 2/24/2024 4:24 AM, Bjorn Helgaas wrote:
> > > > > > On Fri, Feb 23, 2024 at 07:33:38PM +0530, Mrinmay Sarkar wrote:
> > > > > > > Due to some hardware changes, SA8775P has set the
> > > > > > > NO_SNOOP attribute in its TLP for all the PCIe
> > > > > > > controllers. NO_SNOOP attribute when set, the requester
> > > > > > > is indicating that there no cache coherency issues exit
> > > > > > > for the addressed memory on the host i.e., memory is not
> > > > > > > cached. But in reality, requester cannot assume this
> > > > > > > unless there is a complete control/visibility over the
> > > > > > > addressed memory on the host.
> > > > > >
> > > > > > Forgive my ignorance here. It sounds like the cache
> > > > > > coherency issue would refer to system memory, so the
> > > > > > relevant No Snoop attribute would be in DMA transactions,
> > > > > > i.e., Memory Reads or Writes initiated by PCIe Endpoints.
> > > > > > But it looks like this patch would affect TLPs initiated
> > > > > > by the Root Complex, not those from Endpoints, so I'm
> > > > > > confused about how this works.
> > > > > >
> > > > > > If this were in the qcom-ep driver, it would make sense
> > > > > > that setting No Snoop in the TLPs initiated by the
> > > > > > Endpoint could be a problem, but that doesn't seem to be
> > > > > > what this patch is concerned with.
> > > > >
> > > > > I think in multiprocessor system cache coherency issue might
> > > > > occur. and RC as well needs to snoop cache to avoid
> > > > > coherency as it is not enable by default.
> > > >
> > > > My mental picture isn't detailed enough, so I'm still
> > > > confused. We're talking about TLPs initiated by the RC.
> > > > Normally these would be because a driver did a CPU load or
> > > > store to a PCIe device MMIO space, not to system memory.
> > >
> > > Endpoint can expose its system memory as a BAR to the host. In
> > > that case, the cache coherency issue would apply for TLPs
> > > originating from RC as well.
> >
> > What PCIe transactions are involved here? So far I know about:
> >
> > RC initiates Memory Read Request (or Write) with NO_SNOOP==0
> > ...
> > EP responds with Completion with Data (for Read)
>
> The memory on the endpoint may be cached (due to linear map and
> such). So if the RC is initiating the MWd TLP with NO_SNOOP=1, then
> there would be coherency issues because there is no guarantee that
> the memory is not cached on the endpoint. So, not snooping the
> caches and directly writing to the DDR would cause coherency issues
> on the endpoint as well.
I don't know what linear map is, but I'll take your word for it that
endpoints are allowed to cache things internally. So I guess in the
ideal world there might be a way for a driver to specify no-snoop for
accesses to its device if it knows there is no caching.
The commit log for this patch refers to caching on the *host*, though,
and IIUC you're saying this patch clears NO_SNOOP on TLPs from the RC
because of potential coherency issues on the *endpoint*.
> > But I guess you're saying the EP would initiate other transactions in
> > the middle related to snooping? I don't know what those are.
> >
> > > > But I guess you're suggesting the RC can initiate a TLP with a system
> > > > memory address? And this TLP would be routed not to a Root Port or to
> > > > downstream devices, but it would instead be kind of a loopback and be
> > > > routed back up through the RC and maybe IOMMU, to system memory?
> > > >
> > > > I would have expected accesses like this to be routed directly to
> > > > system memory without ever reaching the PCIe RC.
> > > >
> > > > > and we are enabling this feature for qcom-ep driver as well.
> > > > > it is in patch2.
> > > > >
> > > > > Thanks
> > > > > Mrinmay
> > > > >
> > > > > > > And worst case, if the memory is cached on the host, it may lead to
> > > > > > > memory corruption issues. It should be noted that the caching of memory
> > > > > > > on the host is not solely dependent on the NO_SNOOP attribute in TLP.
> > > > > > >
> > > > > > > So to avoid the corruption, this patch overrides the NO_SNOOP attribute
> > > > > > > by setting the PCIE_PARF_NO_SNOOP_OVERIDE register. This patch is not
> > > > > > > needed for other upstream supported platforms since they do not set
> > > > > > > NO_SNOOP attribute by default.
> > > > > > >
> > > > > > > 8775 has IP version 1.34.0 so intruduce a new cfg(cfg_1_34_0) for this
> > > > > > > platform. Assign enable_cache_snoop flag into struct qcom_pcie_cfg and
> > > > > > > set it true in cfg_1_34_0 and enable cache snooping if this particular
> > > > > > > flag is true.
> > > > > > s/intruduce/introduce/
> > > > > >
> > > > > > Bjorn
> > >
> > > --
> > > மணிவண்ணன் சதாசிவம்
>
> --
> மணிவண்ணன் சதாசிவம்
On Wed, Feb 28, 2024 at 01:34:41PM -0600, Bjorn Helgaas wrote:
> On Thu, Feb 29, 2024 at 12:15:02AM +0530, Manivannan Sadhasivam wrote:
> > On Wed, Feb 28, 2024 at 11:39:07AM -0600, Bjorn Helgaas wrote:
> > > On Wed, Feb 28, 2024 at 10:44:12PM +0530, Manivannan Sadhasivam wrote:
> > > > On Wed, Feb 28, 2024 at 09:02:11AM -0600, Bjorn Helgaas wrote:
> > > > > On Wed, Feb 28, 2024 at 06:34:11PM +0530, Mrinmay Sarkar wrote:
> > > > > > On 2/24/2024 4:24 AM, Bjorn Helgaas wrote:
> > > > > > > On Fri, Feb 23, 2024 at 07:33:38PM +0530, Mrinmay Sarkar wrote:
> > > > > > > > Due to some hardware changes, SA8775P has set the
> > > > > > > > NO_SNOOP attribute in its TLP for all the PCIe
> > > > > > > > controllers. NO_SNOOP attribute when set, the requester
> > > > > > > > is indicating that there no cache coherency issues exit
> > > > > > > > for the addressed memory on the host i.e., memory is not
> > > > > > > > cached. But in reality, requester cannot assume this
> > > > > > > > unless there is a complete control/visibility over the
> > > > > > > > addressed memory on the host.
> > > > > > >
> > > > > > > Forgive my ignorance here. It sounds like the cache
> > > > > > > coherency issue would refer to system memory, so the
> > > > > > > relevant No Snoop attribute would be in DMA transactions,
> > > > > > > i.e., Memory Reads or Writes initiated by PCIe Endpoints.
> > > > > > > But it looks like this patch would affect TLPs initiated
> > > > > > > by the Root Complex, not those from Endpoints, so I'm
> > > > > > > confused about how this works.
> > > > > > >
> > > > > > > If this were in the qcom-ep driver, it would make sense
> > > > > > > that setting No Snoop in the TLPs initiated by the
> > > > > > > Endpoint could be a problem, but that doesn't seem to be
> > > > > > > what this patch is concerned with.
> > > > > >
> > > > > > I think in multiprocessor system cache coherency issue might
> > > > > > occur. and RC as well needs to snoop cache to avoid
> > > > > > coherency as it is not enable by default.
> > > > >
> > > > > My mental picture isn't detailed enough, so I'm still
> > > > > confused. We're talking about TLPs initiated by the RC.
> > > > > Normally these would be because a driver did a CPU load or
> > > > > store to a PCIe device MMIO space, not to system memory.
> > > >
> > > > Endpoint can expose its system memory as a BAR to the host. In
> > > > that case, the cache coherency issue would apply for TLPs
> > > > originating from RC as well.
> > >
> > > What PCIe transactions are involved here? So far I know about:
> > >
> > > RC initiates Memory Read Request (or Write) with NO_SNOOP==0
> > > ...
> > > EP responds with Completion with Data (for Read)
> >
> > The memory on the endpoint may be cached (due to linear map and
> > such). So if the RC is initiating the MWd TLP with NO_SNOOP=1, then
> > there would be coherency issues because there is no guarantee that
> > the memory is not cached on the endpoint. So, not snooping the
> > caches and directly writing to the DDR would cause coherency issues
> > on the endpoint as well.
>
> I don't know what linear map is, but I'll take your word for it that
> endpoints are allowed to cache things internally. So I guess in the
> ideal world there might be a way for a driver to specify no-snoop for
> accesses to its device if it knows there is no caching.
>
I referred to Linux kernel's mapping of the DDR space as "linear map". But the
endpoint may not run only Linux, but any RTOS or even bare metal. So it is
certainly possible the BAR memory could be cached.
> The commit log for this patch refers to caching on the *host*, though,
> and IIUC you're saying this patch clears NO_SNOOP on TLPs from the RC
> because of potential coherency issues on the *endpoint*.
>
Yeah, the commit message was wrong. I shared the wording during the review of
previous version and it got duplicated for both RC and EP patches :(
This should be fixed.
- Mani
--
மணிவண்ணன் சதாசிவம்
On Fri, Feb 23, 2024 at 07:33:38PM +0530, Mrinmay Sarkar wrote:
Subject should be:
"PCI: qcom: Override NO_SNOOP attribute for SA8775P"
> Due to some hardware changes, SA8775P has set the NO_SNOOP attribute
> in its TLP for all the PCIe controllers. NO_SNOOP attribute when set,
> the requester is indicating that there no cache coherency issues exit
> for the addressed memory on the host i.e., memory is not cached. But
s/host/endpoint
> in reality, requester cannot assume this unless there is a complete
> control/visibility over the addressed memory on the host.
>
s/host/endpoint
> And worst case, if the memory is cached on the host, it may lead to
s/host/endpoint
> memory corruption issues. It should be noted that the caching of memory
> on the host is not solely dependent on the NO_SNOOP attribute in TLP.
>
s/host/endpoint
> So to avoid the corruption, this patch overrides the NO_SNOOP attribute
> by setting the PCIE_PARF_NO_SNOOP_OVERIDE register. This patch is not
> needed for other upstream supported platforms since they do not set
> NO_SNOOP attribute by default.
>
> 8775 has IP version 1.34.0 so intruduce a new cfg(cfg_1_34_0) for this
> platform. Assign enable_cache_snoop flag into struct qcom_pcie_cfg and
> set it true in cfg_1_34_0 and enable cache snooping if this particular
> flag is true.
>
> Signed-off-by: Mrinmay Sarkar <[email protected]>
> ---
> drivers/pci/controller/dwc/pcie-qcom.c | 20 +++++++++++++++++++-
> 1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 2ce2a3bd932b..872be7f7d7b3 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -51,6 +51,7 @@
> #define PARF_SID_OFFSET 0x234
> #define PARF_BDF_TRANSLATE_CFG 0x24c
> #define PARF_SLV_ADDR_SPACE_SIZE 0x358
> +#define PARF_NO_SNOOP_OVERIDE 0x3d4
> #define PARF_DEVICE_TYPE 0x1000
> #define PARF_BDF_TO_SID_TABLE_N 0x2000
>
> @@ -117,6 +118,10 @@
> /* PARF_LTSSM register fields */
> #define LTSSM_EN BIT(8)
>
> +/* PARF_NO_SNOOP_OVERIDE register fields */
> +#define WR_NO_SNOOP_OVERIDE_EN BIT(1)
> +#define RD_NO_SNOOP_OVERIDE_EN BIT(3)
> +
> /* PARF_DEVICE_TYPE register fields */
> #define DEVICE_TYPE_RC 0x4
>
> @@ -229,6 +234,7 @@ struct qcom_pcie_ops {
>
Please add Kdoc comments for this struct. And describe the "override_no_snoop"
member as below:
"Override NO_SNOOP attribute in TLP to enable cache snooping"
> struct qcom_pcie_cfg {
> const struct qcom_pcie_ops *ops;
> + bool enable_cache_snoop;
Rename this to "override_no_snoop"
> };
>
> struct qcom_pcie {
> @@ -961,6 +967,13 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
>
> static int qcom_pcie_post_init_2_7_0(struct qcom_pcie *pcie)
> {
> + const struct qcom_pcie_cfg *pcie_cfg = pcie->cfg;
> +
> + /* Enable cache snooping for SA8775P */
Remove this comment in favor of Kdoc mentioned above.
- Mani
--
மணிவண்ணன் சதாசிவம்