devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 0/3] arm64: qcom: sa8775p: add cache coherency support for SA8775P
@ 2024-03-06 13:11 Mrinmay Sarkar
  2024-03-06 13:11 ` [PATCH v6 1/3] PCI: qcom: Override NO_SNOOP attribute for SA8775P RC Mrinmay Sarkar
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Mrinmay Sarkar @ 2024-03-06 13:11 UTC (permalink / raw)
  To: andersson, krzysztof.kozlowski+dt, conor+dt, konrad.dybcio,
	manivannan.sadhasivam, robh
  Cc: quic_shazhuss, quic_nitegupt, quic_ramkri, quic_nayiluri,
	quic_krichai, quic_vbadigan, quic_schintav, Mrinmay Sarkar,
	Lorenzo Pieralisi, Krzysztof Wilczyński, Bjorn Helgaas,
	linux-arm-msm, devicetree, linux-kernel, linux-pci

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 no cache coherency issues exist 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/1701432377-16899-1-git-send-email-quic_msarkar@quicinc.com/
https://lore.kernel.org/all/20240306-dw-hdma-v4-4-9fed506e95be@linaro.org/ [1]

V5 -> V6:
- updated commit message as per comments
- added Kdoc comments in patch1
- change variable name from enable_cache_snoop to
  override_no_snoop
- sort reg offset define in patch2

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: Override NO_SNOOP attribute for SA8775P RC
  PCI: qcom-ep: Override NO_SNOOP attribute 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    | 25 ++++++++++++++++++++++++-
 3 files changed, 42 insertions(+), 4 deletions(-)

-- 
2.7.4


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v6 1/3] PCI: qcom: Override NO_SNOOP attribute for SA8775P RC
  2024-03-06 13:11 [PATCH v6 0/3] arm64: qcom: sa8775p: add cache coherency support for SA8775P Mrinmay Sarkar
@ 2024-03-06 13:11 ` Mrinmay Sarkar
  2024-03-10 12:54   ` Manivannan Sadhasivam
  2024-03-06 13:11 ` [PATCH v6 2/3] PCI: qcom-ep: Override NO_SNOOP attribute for SA8775P EP Mrinmay Sarkar
  2024-03-06 13:11 ` [PATCH v6 3/3] arm64: dts: qcom: sa8775p: Mark PCIe EP controller as cache coherent Mrinmay Sarkar
  2 siblings, 1 reply; 7+ messages in thread
From: Mrinmay Sarkar @ 2024-03-06 13:11 UTC (permalink / raw)
  To: andersson, krzysztof.kozlowski+dt, conor+dt, konrad.dybcio,
	manivannan.sadhasivam, robh
  Cc: quic_shazhuss, quic_nitegupt, quic_ramkri, quic_nayiluri,
	quic_krichai, quic_vbadigan, quic_schintav, Mrinmay Sarkar,
	Lorenzo Pieralisi, Krzysztof Wilczyński, Bjorn Helgaas,
	linux-arm-msm, devicetree, linux-kernel, linux-pci

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 no cache coherency issue exist for
the addressed memory on the endpoint 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 endpoint.

And worst case, if the memory is cached on the endpoint, it may lead to
memory corruption issues. It should be noted that the caching of memory
on the endpoint 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 introduce a new cfg(cfg_1_34_0) for this
platform. Assign override_no_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 <quic_msarkar@quicinc.com>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 2ce2a3b..d4c1e69 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
 
@@ -227,8 +232,14 @@ struct qcom_pcie_ops {
 	int (*config_sid)(struct qcom_pcie *pcie);
 };
 
+ /**
+  * struct qcom_pcie_cfg - Per SoC config struct
+  * @ops: qcom pcie ops structure
+  * @override_no_snoop: Override NO_SNOOP attribute in TLP to enable cache snooping
+  */
 struct qcom_pcie_cfg {
 	const struct qcom_pcie_ops *ops;
+	bool override_no_snoop;
 };
 
 struct qcom_pcie {
@@ -961,6 +972,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;
+
+	/* Override NO_SNOOP attribute in TLP to enable cache snooping */
+	if (pcie_cfg->override_no_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 +1352,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,
+	.override_no_snoop = true,
+};
+
 static const struct qcom_pcie_cfg cfg_2_1_0 = {
 	.ops = &ops_2_1_0,
 };
@@ -1630,7 +1653,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.7.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v6 2/3] PCI: qcom-ep: Override NO_SNOOP attribute for SA8775P EP
  2024-03-06 13:11 [PATCH v6 0/3] arm64: qcom: sa8775p: add cache coherency support for SA8775P Mrinmay Sarkar
  2024-03-06 13:11 ` [PATCH v6 1/3] PCI: qcom: Override NO_SNOOP attribute for SA8775P RC Mrinmay Sarkar
@ 2024-03-06 13:11 ` Mrinmay Sarkar
  2024-03-10 12:57   ` Manivannan Sadhasivam
  2024-03-06 13:11 ` [PATCH v6 3/3] arm64: dts: qcom: sa8775p: Mark PCIe EP controller as cache coherent Mrinmay Sarkar
  2 siblings, 1 reply; 7+ messages in thread
From: Mrinmay Sarkar @ 2024-03-06 13:11 UTC (permalink / raw)
  To: andersson, krzysztof.kozlowski+dt, conor+dt, konrad.dybcio,
	manivannan.sadhasivam, robh
  Cc: quic_shazhuss, quic_nitegupt, quic_ramkri, quic_nayiluri,
	quic_krichai, quic_vbadigan, quic_schintav, Mrinmay Sarkar,
	Lorenzo Pieralisi, Krzysztof Wilczyński, Bjorn Helgaas,
	linux-arm-msm, devicetree, linux-kernel, linux-pci

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 no cache coherency issues exist 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 <quic_msarkar@quicinc.com>
---
 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 89d06a3..aa8e979 100644
--- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
+++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
@@ -47,6 +47,7 @@
 #define PARF_DBI_BASE_ADDR_HI			0x354
 #define PARF_SLV_ADDR_SPACE_SIZE		0x358
 #define PARF_SLV_ADDR_SPACE_SIZE_HI		0x35c
+#define PARF_NO_SNOOP_OVERIDE			0x3d4
 #define PARF_ATU_BASE_ADDR			0x634
 #define PARF_ATU_BASE_ADDR_HI			0x638
 #define PARF_SRIS_MODE				0x644
@@ -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
+ * @override_no_snoop: Override NO_SNOOP attribute in TLP to enable cache snooping
  */
 struct qcom_pcie_ep_cfg {
 	bool hdma_support;
+	bool override_no_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);
 
+	/* Override NO_SNOOP attribute in TLP to enable cache snooping */
+	if (pcie_ep->cfg && pcie_ep->cfg->override_no_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,
+	.override_no_snoop = true,
 };
 
 static const struct of_device_id qcom_pcie_ep_match[] = {
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v6 3/3] arm64: dts: qcom: sa8775p: Mark PCIe EP controller as cache coherent
  2024-03-06 13:11 [PATCH v6 0/3] arm64: qcom: sa8775p: add cache coherency support for SA8775P Mrinmay Sarkar
  2024-03-06 13:11 ` [PATCH v6 1/3] PCI: qcom: Override NO_SNOOP attribute for SA8775P RC Mrinmay Sarkar
  2024-03-06 13:11 ` [PATCH v6 2/3] PCI: qcom-ep: Override NO_SNOOP attribute for SA8775P EP Mrinmay Sarkar
@ 2024-03-06 13:11 ` Mrinmay Sarkar
  2024-03-06 16:12   ` Krzysztof Kozlowski
  2 siblings, 1 reply; 7+ messages in thread
From: Mrinmay Sarkar @ 2024-03-06 13:11 UTC (permalink / raw)
  To: andersson, krzysztof.kozlowski+dt, conor+dt, konrad.dybcio,
	manivannan.sadhasivam, robh
  Cc: quic_shazhuss, quic_nitegupt, quic_ramkri, quic_nayiluri,
	quic_krichai, quic_vbadigan, quic_schintav, Mrinmay Sarkar,
	Lorenzo Pieralisi, Krzysztof Wilczyński, Bjorn Helgaas,
	linux-arm-msm, devicetree, linux-kernel, linux-pci

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 <quic_msarkar@quicinc.com>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 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 d9802027..53c31c7 100644
--- a/arch/arm64/boot/dts/qcom/sa8775p.dtsi
+++ b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
@@ -3713,6 +3713,7 @@
 				<&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.7.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v6 3/3] arm64: dts: qcom: sa8775p: Mark PCIe EP controller as cache coherent
  2024-03-06 13:11 ` [PATCH v6 3/3] arm64: dts: qcom: sa8775p: Mark PCIe EP controller as cache coherent Mrinmay Sarkar
@ 2024-03-06 16:12   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 7+ messages in thread
From: Krzysztof Kozlowski @ 2024-03-06 16:12 UTC (permalink / raw)
  To: Mrinmay Sarkar, andersson, krzysztof.kozlowski+dt, conor+dt,
	konrad.dybcio, manivannan.sadhasivam, robh
  Cc: quic_shazhuss, quic_nitegupt, quic_ramkri, quic_nayiluri,
	quic_krichai, quic_vbadigan, quic_schintav, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Bjorn Helgaas, linux-arm-msm,
	devicetree, linux-kernel, linux-pci

On 06/03/2024 14:11, Mrinmay Sarkar wrote:
> 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 <quic_msarkar@quicinc.com>
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v6 1/3] PCI: qcom: Override NO_SNOOP attribute for SA8775P RC
  2024-03-06 13:11 ` [PATCH v6 1/3] PCI: qcom: Override NO_SNOOP attribute for SA8775P RC Mrinmay Sarkar
@ 2024-03-10 12:54   ` Manivannan Sadhasivam
  0 siblings, 0 replies; 7+ messages in thread
From: Manivannan Sadhasivam @ 2024-03-10 12:54 UTC (permalink / raw)
  To: Mrinmay Sarkar
  Cc: andersson, krzysztof.kozlowski+dt, conor+dt, konrad.dybcio, robh,
	quic_shazhuss, quic_nitegupt, quic_ramkri, quic_nayiluri,
	quic_krichai, quic_vbadigan, quic_schintav, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Bjorn Helgaas, linux-arm-msm,
	devicetree, linux-kernel, linux-pci

On Wed, Mar 06, 2024 at 06:41:10PM +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 no cache coherency issue exist for
> the addressed memory on the endpoint 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 endpoint.
> 
> And worst case, if the memory is cached on the endpoint, it may lead to
> memory corruption issues. It should be noted that the caching of memory
> on the endpoint 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 introduce a new cfg(cfg_1_34_0) for this
> platform. Assign override_no_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 <quic_msarkar@quicinc.com>

Minor nit below. With that addressed,

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 25 ++++++++++++++++++++++++-
>  1 file changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 2ce2a3b..d4c1e69 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
>  
> @@ -227,8 +232,14 @@ struct qcom_pcie_ops {
>  	int (*config_sid)(struct qcom_pcie *pcie);
>  };
>  
> + /**
> +  * struct qcom_pcie_cfg - Per SoC config struct
> +  * @ops: qcom pcie ops structure
> +  * @override_no_snoop: Override NO_SNOOP attribute in TLP to enable cache snooping
> +  */
>  struct qcom_pcie_cfg {
>  	const struct qcom_pcie_ops *ops;
> +	bool override_no_snoop;
>  };
>  
>  struct qcom_pcie {
> @@ -961,6 +972,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;
> +
> +	/* Override NO_SNOOP attribute in TLP to enable cache snooping */

This comment is now redundant due to Kdoc of override_no_snoop.

- Mani

> +	if (pcie_cfg->override_no_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 +1352,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,
> +	.override_no_snoop = true,
> +};
> +
>  static const struct qcom_pcie_cfg cfg_2_1_0 = {
>  	.ops = &ops_2_1_0,
>  };
> @@ -1630,7 +1653,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.7.4
> 

-- 
மணிவண்ணன் சதாசிவம்

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v6 2/3] PCI: qcom-ep: Override NO_SNOOP attribute for SA8775P EP
  2024-03-06 13:11 ` [PATCH v6 2/3] PCI: qcom-ep: Override NO_SNOOP attribute for SA8775P EP Mrinmay Sarkar
@ 2024-03-10 12:57   ` Manivannan Sadhasivam
  0 siblings, 0 replies; 7+ messages in thread
From: Manivannan Sadhasivam @ 2024-03-10 12:57 UTC (permalink / raw)
  To: Mrinmay Sarkar
  Cc: andersson, krzysztof.kozlowski+dt, conor+dt, konrad.dybcio, robh,
	quic_shazhuss, quic_nitegupt, quic_ramkri, quic_nayiluri,
	quic_krichai, quic_vbadigan, quic_schintav, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Bjorn Helgaas, linux-arm-msm,
	devicetree, linux-kernel, linux-pci

On Wed, Mar 06, 2024 at 06:41:11PM +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 no cache coherency issues exist 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 <quic_msarkar@quicinc.com>

Same nit as previous patch. With that addressed,

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

> ---
>  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 89d06a3..aa8e979 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> @@ -47,6 +47,7 @@
>  #define PARF_DBI_BASE_ADDR_HI			0x354
>  #define PARF_SLV_ADDR_SPACE_SIZE		0x358
>  #define PARF_SLV_ADDR_SPACE_SIZE_HI		0x35c
> +#define PARF_NO_SNOOP_OVERIDE			0x3d4
>  #define PARF_ATU_BASE_ADDR			0x634
>  #define PARF_ATU_BASE_ADDR_HI			0x638
>  #define PARF_SRIS_MODE				0x644
> @@ -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
> + * @override_no_snoop: Override NO_SNOOP attribute in TLP to enable cache snooping
>   */
>  struct qcom_pcie_ep_cfg {
>  	bool hdma_support;
> +	bool override_no_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);
>  
> +	/* Override NO_SNOOP attribute in TLP to enable cache snooping */

Same comment as previous patch.

- Mani

> +	if (pcie_ep->cfg && pcie_ep->cfg->override_no_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,
> +	.override_no_snoop = true,
>  };
>  
>  static const struct of_device_id qcom_pcie_ep_match[] = {
> -- 
> 2.7.4
> 

-- 
மணிவண்ணன் சதாசிவம்

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2024-03-10 12:57 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-06 13:11 [PATCH v6 0/3] arm64: qcom: sa8775p: add cache coherency support for SA8775P Mrinmay Sarkar
2024-03-06 13:11 ` [PATCH v6 1/3] PCI: qcom: Override NO_SNOOP attribute for SA8775P RC Mrinmay Sarkar
2024-03-10 12:54   ` Manivannan Sadhasivam
2024-03-06 13:11 ` [PATCH v6 2/3] PCI: qcom-ep: Override NO_SNOOP attribute for SA8775P EP Mrinmay Sarkar
2024-03-10 12:57   ` Manivannan Sadhasivam
2024-03-06 13:11 ` [PATCH v6 3/3] arm64: dts: qcom: sa8775p: Mark PCIe EP controller as cache coherent Mrinmay Sarkar
2024-03-06 16:12   ` Krzysztof Kozlowski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).