[PATCH v2 2/8] PCI: qcom: Disable write access to read only registers for IP v2.9.0

Manivannan Sadhasivam posted 8 patches 2 years, 8 months ago
There is a newer version of this series
[PATCH v2 2/8] PCI: qcom: Disable write access to read only registers for IP v2.9.0
Posted by Manivannan Sadhasivam 2 years, 8 months ago
In the post init sequence of v2.9.0, write access to read only registers
are not disabled after updating the registers. Fix it by disabling the
access after register update.

Fixes: 0cf7c2efe8ac ("PCI: qcom: Add IPQ60xx support")
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@gmail.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 01795ee7ce45..391a45d1e70a 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -1136,6 +1136,7 @@ static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie)
 	writel(0, pcie->parf + PARF_Q2A_FLUSH);
 
 	dw_pcie_dbi_ro_wr_en(pci);
+
 	writel(PCIE_CAP_SLOT_VAL, pci->dbi_base + offset + PCI_EXP_SLTCAP);
 
 	val = readl(pci->dbi_base + offset + PCI_EXP_LNKCAP);
@@ -1145,6 +1146,8 @@ static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie)
 	writel(PCI_EXP_DEVCTL2_COMP_TMOUT_DIS, pci->dbi_base + offset +
 			PCI_EXP_DEVCTL2);
 
+	dw_pcie_dbi_ro_wr_dis(pci);
+
 	for (i = 0; i < 256; i++)
 		writel(0, pcie->parf + PARF_BDF_TO_SID_TABLE_N + (4 * i));
 
-- 
2.25.1
Re: [PATCH v2 2/8] PCI: qcom: Disable write access to read only registers for IP v2.9.0
Posted by Dmitry Baryshkov 2 years, 8 months ago
On Fri, 19 May 2023 at 17:31, Manivannan Sadhasivam
<manivannan.sadhasivam@linaro.org> wrote:
>
> In the post init sequence of v2.9.0, write access to read only registers
> are not disabled after updating the registers. Fix it by disabling the
> access after register update.
>
> Fixes: 0cf7c2efe8ac ("PCI: qcom: Add IPQ60xx support")
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@gmail.com>

Could you please drop the @gmail R-B tags? I was mistaken when sending
them (I'm not even sure that this email exists).

> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 01795ee7ce45..391a45d1e70a 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -1136,6 +1136,7 @@ static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie)
>         writel(0, pcie->parf + PARF_Q2A_FLUSH);
>
>         dw_pcie_dbi_ro_wr_en(pci);
> +
>         writel(PCIE_CAP_SLOT_VAL, pci->dbi_base + offset + PCI_EXP_SLTCAP);
>
>         val = readl(pci->dbi_base + offset + PCI_EXP_LNKCAP);
> @@ -1145,6 +1146,8 @@ static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie)
>         writel(PCI_EXP_DEVCTL2_COMP_TMOUT_DIS, pci->dbi_base + offset +
>                         PCI_EXP_DEVCTL2);
>
> +       dw_pcie_dbi_ro_wr_dis(pci);
> +
>         for (i = 0; i < 256; i++)
>                 writel(0, pcie->parf + PARF_BDF_TO_SID_TABLE_N + (4 * i));
>
> --
> 2.25.1
>


-- 
With best wishes
Dmitry
Re: [PATCH v2 2/8] PCI: qcom: Disable write access to read only registers for IP v2.9.0
Posted by Manivannan Sadhasivam 2 years, 8 months ago
On Mon, May 22, 2023 at 04:00:51PM +0300, Dmitry Baryshkov wrote:
> On Fri, 19 May 2023 at 17:31, Manivannan Sadhasivam
> <manivannan.sadhasivam@linaro.org> wrote:
> >
> > In the post init sequence of v2.9.0, write access to read only registers
> > are not disabled after updating the registers. Fix it by disabling the
> > access after register update.
> >
> > Fixes: 0cf7c2efe8ac ("PCI: qcom: Add IPQ60xx support")
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@gmail.com>
> 
> Could you please drop the @gmail R-B tags? I was mistaken when sending
> them (I'm not even sure that this email exists).
> 

b4 picked them up. Will drop this tag from all patches.

- Mani

> > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > ---
> >  drivers/pci/controller/dwc/pcie-qcom.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> > index 01795ee7ce45..391a45d1e70a 100644
> > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > @@ -1136,6 +1136,7 @@ static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie)
> >         writel(0, pcie->parf + PARF_Q2A_FLUSH);
> >
> >         dw_pcie_dbi_ro_wr_en(pci);
> > +
> >         writel(PCIE_CAP_SLOT_VAL, pci->dbi_base + offset + PCI_EXP_SLTCAP);
> >
> >         val = readl(pci->dbi_base + offset + PCI_EXP_LNKCAP);
> > @@ -1145,6 +1146,8 @@ static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie)
> >         writel(PCI_EXP_DEVCTL2_COMP_TMOUT_DIS, pci->dbi_base + offset +
> >                         PCI_EXP_DEVCTL2);
> >
> > +       dw_pcie_dbi_ro_wr_dis(pci);
> > +
> >         for (i = 0; i < 256; i++)
> >                 writel(0, pcie->parf + PARF_BDF_TO_SID_TABLE_N + (4 * i));
> >
> > --
> > 2.25.1
> >
> 
> 
> -- 
> With best wishes
> Dmitry

-- 
மணிவண்ணன் சதாசிவம்
Re: [PATCH v2 2/8] PCI: qcom: Disable write access to read only registers for IP v2.9.0
Posted by Lorenzo Pieralisi 2 years, 8 months ago
On Fri, May 19, 2023 at 08:01:11PM +0530, Manivannan Sadhasivam wrote:
> In the post init sequence of v2.9.0, write access to read only registers
> are not disabled after updating the registers. Fix it by disabling the
> access after register update.
> 
> Fixes: 0cf7c2efe8ac ("PCI: qcom: Add IPQ60xx support")
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@gmail.com>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 01795ee7ce45..391a45d1e70a 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -1136,6 +1136,7 @@ static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie)
>  	writel(0, pcie->parf + PARF_Q2A_FLUSH);
>  
>  	dw_pcie_dbi_ro_wr_en(pci);
> +

Nit: spurious change.

Lorenzo

>  	writel(PCIE_CAP_SLOT_VAL, pci->dbi_base + offset + PCI_EXP_SLTCAP);
>  
>  	val = readl(pci->dbi_base + offset + PCI_EXP_LNKCAP);
> @@ -1145,6 +1146,8 @@ static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie)
>  	writel(PCI_EXP_DEVCTL2_COMP_TMOUT_DIS, pci->dbi_base + offset +
>  			PCI_EXP_DEVCTL2);
>  
> +	dw_pcie_dbi_ro_wr_dis(pci);
> +
>  	for (i = 0; i < 256; i++)
>  		writel(0, pcie->parf + PARF_BDF_TO_SID_TABLE_N + (4 * i));
>  
> -- 
> 2.25.1
>
Re: [PATCH v2 2/8] PCI: qcom: Disable write access to read only registers for IP v2.9.0
Posted by Manivannan Sadhasivam 2 years, 8 months ago
On Mon, May 22, 2023 at 10:56:19AM +0200, Lorenzo Pieralisi wrote:
> On Fri, May 19, 2023 at 08:01:11PM +0530, Manivannan Sadhasivam wrote:
> > In the post init sequence of v2.9.0, write access to read only registers
> > are not disabled after updating the registers. Fix it by disabling the
> > access after register update.
> > 
> > Fixes: 0cf7c2efe8ac ("PCI: qcom: Add IPQ60xx support")
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@gmail.com>
> > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > ---
> >  drivers/pci/controller/dwc/pcie-qcom.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> > index 01795ee7ce45..391a45d1e70a 100644
> > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > @@ -1136,6 +1136,7 @@ static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie)
> >  	writel(0, pcie->parf + PARF_Q2A_FLUSH);
> >  
> >  	dw_pcie_dbi_ro_wr_en(pci);
> > +
> 
> Nit: spurious change.
> 

Well that's intentional. It's good to have a newline between these guard
functions to differentiate them from the DBI accesses. We do it in other places
in the driver.

But I thought this change doesn't warrant a mention in commit message or a
separate patch.

Let me know otherwise.

- Mani

> Lorenzo
> 
> >  	writel(PCIE_CAP_SLOT_VAL, pci->dbi_base + offset + PCI_EXP_SLTCAP);
> >  
> >  	val = readl(pci->dbi_base + offset + PCI_EXP_LNKCAP);
> > @@ -1145,6 +1146,8 @@ static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie)
> >  	writel(PCI_EXP_DEVCTL2_COMP_TMOUT_DIS, pci->dbi_base + offset +
> >  			PCI_EXP_DEVCTL2);
> >  
> > +	dw_pcie_dbi_ro_wr_dis(pci);
> > +
> >  	for (i = 0; i < 256; i++)
> >  		writel(0, pcie->parf + PARF_BDF_TO_SID_TABLE_N + (4 * i));
> >  
> > -- 
> > 2.25.1
> > 

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