| Message ID | 1700051821-1087-2-git-send-email-quic_msarkar@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | arm64: qcom: sa8775p: add cache coherency support for SA8775P | expand |
On Wed, 15 Nov 2023 at 14:37, Mrinmay Sarkar <quic_msarkar@quicinc.com> wrote: > > This change will enable cache snooping logic to support > cache coherency for 8775 RC platform. > > Signed-off-by: Mrinmay Sarkar <quic_msarkar@quicinc.com> > --- > drivers/pci/controller/dwc/pcie-qcom.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c > index 6902e97..b82ccd1 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 PCIE_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 > > @@ -961,6 +966,14 @@ 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) > { > + struct dw_pcie *pci = pcie->pci; > + struct device *dev = pci->dev; > + > + /* Enable cache snooping for SA8775P */ > + if (of_device_is_compatible(dev->of_node, "qcom,pcie-sa8775p")) Quoting my feedback from v1: Obviously: please populate a flag in the data structures instead of doing of_device_is_compatible(). Same applies to the patch 2. > + writel(WR_NO_SNOOP_OVERIDE_EN | RD_NO_SNOOP_OVERIDE_EN, > + pcie->parf + PCIE_PARF_NO_SNOOP_OVERIDE); > + > qcom_pcie_clear_hpc(pcie->pci); > > return 0; > -- > 2.7.4 >
On Wed, 15 Nov 2023 at 15:18, Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > > On Wed, 15 Nov 2023 at 14:37, Mrinmay Sarkar <quic_msarkar@quicinc.com> wrote: > > > > This change will enable cache snooping logic to support > > cache coherency for 8775 RC platform. > > > > Signed-off-by: Mrinmay Sarkar <quic_msarkar@quicinc.com> > > --- > > drivers/pci/controller/dwc/pcie-qcom.c | 13 +++++++++++++ > > 1 file changed, 13 insertions(+) > > > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c > > index 6902e97..b82ccd1 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 PCIE_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 > > > > @@ -961,6 +966,14 @@ 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) > > { > > + struct dw_pcie *pci = pcie->pci; > > + struct device *dev = pci->dev; > > + > > + /* Enable cache snooping for SA8775P */ > > + if (of_device_is_compatible(dev->of_node, "qcom,pcie-sa8775p")) > > Quoting my feedback from v1: > > Obviously: please populate a flag in the data structures instead of > doing of_device_is_compatible(). Same applies to the patch 2. Mani, I saw your response for the v1, but I forgot to respond. In my opinion, it's better to have the flag now, even if it is just for a single platform. It allows us to follow the logic of the driver and saves few string ops. > > > > + writel(WR_NO_SNOOP_OVERIDE_EN | RD_NO_SNOOP_OVERIDE_EN, > > + pcie->parf + PCIE_PARF_NO_SNOOP_OVERIDE); > > + > > qcom_pcie_clear_hpc(pcie->pci); > > > > return 0; > > -- > > 2.7.4 > > > > > -- > With best wishes > Dmitry
On Wed, Nov 15, 2023 at 03:21:26PM +0200, Dmitry Baryshkov wrote: > On Wed, 15 Nov 2023 at 15:18, Dmitry Baryshkov > <dmitry.baryshkov@linaro.org> wrote: > > > > On Wed, 15 Nov 2023 at 14:37, Mrinmay Sarkar <quic_msarkar@quicinc.com> wrote: > > > > > > This change will enable cache snooping logic to support > > > cache coherency for 8775 RC platform. > > > > > > Signed-off-by: Mrinmay Sarkar <quic_msarkar@quicinc.com> > > > --- > > > drivers/pci/controller/dwc/pcie-qcom.c | 13 +++++++++++++ > > > 1 file changed, 13 insertions(+) > > > > > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c > > > index 6902e97..b82ccd1 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 PCIE_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 > > > > > > @@ -961,6 +966,14 @@ 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) > > > { > > > + struct dw_pcie *pci = pcie->pci; > > > + struct device *dev = pci->dev; > > > + > > > + /* Enable cache snooping for SA8775P */ > > > + if (of_device_is_compatible(dev->of_node, "qcom,pcie-sa8775p")) > > > > Quoting my feedback from v1: > > > > Obviously: please populate a flag in the data structures instead of > > doing of_device_is_compatible(). Same applies to the patch 2. > > Mani, I saw your response for the v1, but I forgot to respond. In my > opinion, it's better to have the flag now, even if it is just for a > single platform. It allows us to follow the logic of the driver and > saves few string ops. > Ok, I do not have a strong opinion on this. - Mani > > > > > > > + writel(WR_NO_SNOOP_OVERIDE_EN | RD_NO_SNOOP_OVERIDE_EN, > > > + pcie->parf + PCIE_PARF_NO_SNOOP_OVERIDE); > > > + > > > qcom_pcie_clear_hpc(pcie->pci); > > > > > > return 0; > > > -- > > > 2.7.4 > > > > > > > > > -- > > With best wishes > > Dmitry > > > > -- > With best wishes > Dmitry
On Wed, Nov 15, 2023 at 06:06:59PM +0530, Mrinmay Sarkar wrote: > This change will enable cache snooping logic to support > cache coherency for 8775 RC platform. > Please add information on why the cache snoop logic is enabled only on this platform. You have added info in the cover letter, but that's not going to be part of the git history. - Mani > Signed-off-by: Mrinmay Sarkar <quic_msarkar@quicinc.com> > --- > drivers/pci/controller/dwc/pcie-qcom.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c > index 6902e97..b82ccd1 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 PCIE_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 > > @@ -961,6 +966,14 @@ 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) > { > + struct dw_pcie *pci = pcie->pci; > + struct device *dev = pci->dev; > + > + /* Enable cache snooping for SA8775P */ > + if (of_device_is_compatible(dev->of_node, "qcom,pcie-sa8775p")) > + writel(WR_NO_SNOOP_OVERIDE_EN | RD_NO_SNOOP_OVERIDE_EN, > + pcie->parf + PCIE_PARF_NO_SNOOP_OVERIDE); > + > qcom_pcie_clear_hpc(pcie->pci); > > return 0; > -- > 2.7.4 >
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c index 6902e97..b82ccd1 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 PCIE_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 @@ -961,6 +966,14 @@ 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) { + struct dw_pcie *pci = pcie->pci; + struct device *dev = pci->dev; + + /* Enable cache snooping for SA8775P */ + if (of_device_is_compatible(dev->of_node, "qcom,pcie-sa8775p")) + writel(WR_NO_SNOOP_OVERIDE_EN | RD_NO_SNOOP_OVERIDE_EN, + pcie->parf + PCIE_PARF_NO_SNOOP_OVERIDE); + qcom_pcie_clear_hpc(pcie->pci); return 0;
This change will enable cache snooping logic to support cache coherency for 8775 RC platform. Signed-off-by: Mrinmay Sarkar <quic_msarkar@quicinc.com> --- drivers/pci/controller/dwc/pcie-qcom.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)