[PATCH v2 0/2] PCI: qcom: fixup PCIe support on sm8250

[PATCH v2 0/2] PCI: qcom: fixup PCIe support on sm8250

[Dmitry Baryshkov]

SM8250 SoC requires another clock to be up to power up the translation
unit. Add necessary bindings and driver support.

[PATCH v2 1/2] dt-bindings: pci: qcom: Document ddrss_sf_tbu clock for sm8250

[Dmitry Baryshkov]

On SM8250 additional clock is required for PCIe devices to access NOC.
Document this clock in devicetree bindings.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Fixes: 458168247ccc ("dt-bindings: pci: qcom: Document PCIe bindings for SM8250 SoC")
---
 Documentation/devicetree/bindings/pci/qcom,pcie.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
index 3b55310390a0..c87806f76a43 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
@@ -142,6 +142,7 @@
 			- "bus_slave"	Slave AXI clock
 			- "slave_q2a"	Slave Q2A clock
 			- "tbu"		PCIe TBU clock
+			- "ddrss_sf_tbu" PCIe SF TBU clock, required on sm8250
 			- "pipe"	PIPE clock
 
 - resets:
-- 
2.29.2

[PATCH v2 2/2] PCI: qcom: add support for ddrss_sf_tbu clock

[Dmitry Baryshkov]

On SM8250 additional clock is required for PCIe devices to access NOC.
Update PCIe controller driver to control this clock.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Fixes: e1dd639e374a ("PCI: qcom: Add SM8250 SoC support")
---
 drivers/pci/controller/dwc/pcie-qcom.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index affa2713bf80..658d007a764c 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -159,8 +159,9 @@ struct qcom_pcie_resources_2_3_3 {
 	struct reset_control *rst[7];
 };
 
+#define QCOM_PCIE_2_7_0_MAX_CLOCKS	6
 struct qcom_pcie_resources_2_7_0 {
-	struct clk_bulk_data clks[6];
+	struct clk_bulk_data clks[QCOM_PCIE_2_7_0_MAX_CLOCKS + 1]; /* + 1 for sf_tbu */
 	struct regulator_bulk_data supplies[2];
 	struct reset_control *pci_reset;
 	struct clk *pipe_clk;
@@ -1153,10 +1154,15 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie)
 	res->clks[4].id = "slave_q2a";
 	res->clks[5].id = "tbu";
 
-	ret = devm_clk_bulk_get(dev, ARRAY_SIZE(res->clks), res->clks);
+	ret = devm_clk_bulk_get(dev, QCOM_PCIE_2_7_0_MAX_CLOCKS, res->clks);
 	if (ret < 0)
 		return ret;
 
+	/* Optional clock for SM8250 */
+	res->clks[6].clk = devm_clk_get_optional(dev, "ddrss_sf_tbu");
+	if (IS_ERR(res->clks[6].clk))
+		return PTR_ERR(res->clks[6].clk);
+
 	res->pipe_clk = devm_clk_get(dev, "pipe");
 	return PTR_ERR_OR_ZERO(res->pipe_clk);
 }
-- 
2.29.2

Re: [PATCH v2 2/2] PCI: qcom: add support for ddrss_sf_tbu clock

[Manivannan Sadhasivam]

On Wed, Dec 30, 2020 at 02:54:08PM +0300, Dmitry Baryshkov wrote:
> On SM8250 additional clock is required for PCIe devices to access NOC.
> Update PCIe controller driver to control this clock.
> 

If it is really required then why make it optional?

Thanks,
Mani

> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Fixes: e1dd639e374a ("PCI: qcom: Add SM8250 SoC support")
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index affa2713bf80..658d007a764c 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -159,8 +159,9 @@ struct qcom_pcie_resources_2_3_3 {
>  	struct reset_control *rst[7];
>  };
>  
> +#define QCOM_PCIE_2_7_0_MAX_CLOCKS	6
>  struct qcom_pcie_resources_2_7_0 {
> -	struct clk_bulk_data clks[6];
> +	struct clk_bulk_data clks[QCOM_PCIE_2_7_0_MAX_CLOCKS + 1]; /* + 1 for sf_tbu */
>  	struct regulator_bulk_data supplies[2];
>  	struct reset_control *pci_reset;
>  	struct clk *pipe_clk;
> @@ -1153,10 +1154,15 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie)
>  	res->clks[4].id = "slave_q2a";
>  	res->clks[5].id = "tbu";
>  
> -	ret = devm_clk_bulk_get(dev, ARRAY_SIZE(res->clks), res->clks);
> +	ret = devm_clk_bulk_get(dev, QCOM_PCIE_2_7_0_MAX_CLOCKS, res->clks);
>  	if (ret < 0)
>  		return ret;
>  
> +	/* Optional clock for SM8250 */
> +	res->clks[6].clk = devm_clk_get_optional(dev, "ddrss_sf_tbu");
> +	if (IS_ERR(res->clks[6].clk))
> +		return PTR_ERR(res->clks[6].clk);
> +
>  	res->pipe_clk = devm_clk_get(dev, "pipe");
>  	return PTR_ERR_OR_ZERO(res->pipe_clk);
>  }
> -- 
> 2.29.2
> 

Re: [PATCH v2 0/2] PCI: qcom: fixup PCIe support on sm8250

[Manivannan Sadhasivam]

On Wed, Dec 30, 2020 at 02:54:06PM +0300, Dmitry Baryshkov wrote:
> SM8250 SoC requires another clock to be up to power up the translation
> unit. Add necessary bindings and driver support.
> 

So what is the exact issue you're facing?

Thanks,
Mani

> 

Re: [PATCH v2 0/2] PCI: qcom: fixup PCIe support on sm8250

[Dmitry Baryshkov]

Hi Mani,

On Wed, 30 Dec 2020 at 15:35, Manivannan Sadhasivam
<manivannan.sadhasivam@linaro.org> wrote:
>
> On Wed, Dec 30, 2020 at 02:54:06PM +0300, Dmitry Baryshkov wrote:
> > SM8250 SoC requires another clock to be up to power up the translation
> > unit. Add necessary bindings and driver support.
> >
>
> So what is the exact issue you're facing?

IOMMU timeouts for PCIe0 device (WiFi)

-- 
With best wishes
Dmitry

Re: [PATCH v2 0/2] PCI: qcom: fixup PCIe support on sm8250

[Manivannan Sadhasivam]

On Wed, Dec 30, 2020 at 03:38:12PM +0300, Dmitry Baryshkov wrote:
> Hi Mani,
> 
> On Wed, 30 Dec 2020 at 15:35, Manivannan Sadhasivam
> <manivannan.sadhasivam@linaro.org> wrote:
> >
> > On Wed, Dec 30, 2020 at 02:54:06PM +0300, Dmitry Baryshkov wrote:
> > > SM8250 SoC requires another clock to be up to power up the translation
> > > unit. Add necessary bindings and driver support.
> > >
> >
> > So what is the exact issue you're facing?
> 
> IOMMU timeouts for PCIe0 device (WiFi)
> 

Strange. I never observed this issue while testing with onboard QCA6390. Is it
only happening on v5.11?

Thanks,
Mani

> -- 
> With best wishes
> Dmitry

Re: [PATCH v2 0/2] PCI: qcom: fixup PCIe support on sm8250

[Dmitry Baryshkov]

On Wed, 30 Dec 2020 at 15:46, Manivannan Sadhasivam
<manivannan.sadhasivam@linaro.org> wrote:
>
> On Wed, Dec 30, 2020 at 03:38:12PM +0300, Dmitry Baryshkov wrote:
> > Hi Mani,
> >
> > On Wed, 30 Dec 2020 at 15:35, Manivannan Sadhasivam
> > <manivannan.sadhasivam@linaro.org> wrote:
> > >
> > > On Wed, Dec 30, 2020 at 02:54:06PM +0300, Dmitry Baryshkov wrote:
> > > > SM8250 SoC requires another clock to be up to power up the translation
> > > > unit. Add necessary bindings and driver support.
> > > >
> > >
> > > So what is the exact issue you're facing?
> >
> > IOMMU timeouts for PCIe0 device (WiFi)
> >
>
> Strange. I never observed this issue while testing with onboard QCA6390. Is it
> only happening on v5.11?

No, I've faced it with 5.10 also. Don't remember about 5.9. Downstream
4.19 also has this patch.
It well might be that on your board the firmware enables this clock.
However to be on a safe side I think we should enable it too.

-- 
With best wishes
Dmitry

Re: [PATCH v2 0/2] PCI: qcom: fixup PCIe support on sm8250

[Manivannan Sadhasivam]

On Wed, Dec 30, 2020 at 03:57:54PM +0300, Dmitry Baryshkov wrote:
> On Wed, 30 Dec 2020 at 15:46, Manivannan Sadhasivam
> <manivannan.sadhasivam@linaro.org> wrote:
> >
> > On Wed, Dec 30, 2020 at 03:38:12PM +0300, Dmitry Baryshkov wrote:
> > > Hi Mani,
> > >
> > > On Wed, 30 Dec 2020 at 15:35, Manivannan Sadhasivam
> > > <manivannan.sadhasivam@linaro.org> wrote:
> > > >
> > > > On Wed, Dec 30, 2020 at 02:54:06PM +0300, Dmitry Baryshkov wrote:
> > > > > SM8250 SoC requires another clock to be up to power up the translation
> > > > > unit. Add necessary bindings and driver support.
> > > > >
> > > >
> > > > So what is the exact issue you're facing?
> > >
> > > IOMMU timeouts for PCIe0 device (WiFi)
> > >
> >
> > Strange. I never observed this issue while testing with onboard QCA6390. Is it
> > only happening on v5.11?
> 
> No, I've faced it with 5.10 also. Don't remember about 5.9. Downstream
> 4.19 also has this patch.
> It well might be that on your board the firmware enables this clock.
> However to be on a safe side I think we should enable it too.
> 

Okay, then please remove the optional field and make it as a required one.

Thanks,
Mani

> -- 
> With best wishes
> Dmitry

Re: [PATCH v2 0/2] PCI: qcom: fixup PCIe support on sm8250

[Dmitry Baryshkov]

Hi Mani,

On Wed, 30 Dec 2020 at 16:06, Manivannan Sadhasivam
<manivannan.sadhasivam@linaro.org> wrote:
>
> On Wed, Dec 30, 2020 at 03:57:54PM +0300, Dmitry Baryshkov wrote:
> > On Wed, 30 Dec 2020 at 15:46, Manivannan Sadhasivam
> > <manivannan.sadhasivam@linaro.org> wrote:
> > >
> > > On Wed, Dec 30, 2020 at 03:38:12PM +0300, Dmitry Baryshkov wrote:
> > > > Hi Mani,
> > > >
> > > > On Wed, 30 Dec 2020 at 15:35, Manivannan Sadhasivam
> > > > <manivannan.sadhasivam@linaro.org> wrote:
> > > > >
> > > > > On Wed, Dec 30, 2020 at 02:54:06PM +0300, Dmitry Baryshkov wrote:
> > > > > > SM8250 SoC requires another clock to be up to power up the translation
> > > > > > unit. Add necessary bindings and driver support.
> > > > > >
> > > > >
> > > > > So what is the exact issue you're facing?
> > > >
> > > > IOMMU timeouts for PCIe0 device (WiFi)
> > > >
> > >
> > > Strange. I never observed this issue while testing with onboard QCA6390. Is it
> > > only happening on v5.11?
> >
> > No, I've faced it with 5.10 also. Don't remember about 5.9. Downstream
> > 4.19 also has this patch.
> > It well might be that on your board the firmware enables this clock.
> > However to be on a safe side I think we should enable it too.
> >
>
> Okay, then please remove the optional field and make it as a required one.

Done.

-- 
With best wishes
Dmitry