[PATCH v2 00/11] PCI: qcom: add support for PCIe on SM8450 platform

[PATCH v2 00/11] PCI: qcom: add support for PCIe on SM8450 platform

[Dmitry Baryshkov]

There are two different PCIe controllers and PHYs on SM8450, one having
one lane and another with two lanes. Add support for both PCIe
controllers.

After comparing upstream and downstream Qualcomm PCIe drivers, change
the way the driver works with the pipe_clk_src multiplexing.

The clock should be switched to using ref_clk (TCXO) as a parent before
turning the PCIE_x_GDSC power domain off and can be switched to using
PHY's pipe_clk after this power domain is turned on.

Downstream driver uses regulators for the GDSC, so current approach also
(incorrectly) ties pipe_clk multiplexing to regulator
enablement/disablement. However upstream driver uses power-domain and
so GDSC is maintained using pm_runtime_foo() calls. Previous iteration
of this patchset changed the order of operations to call
pm_runtime_foo(). This iteration changes apprach and moves pipe clock
source handling to the GDSC instead. This way the drivers are sure that
the clock is fed from the correct source.

Changes since v1:
 - Merge two patchsets
 - Move pipe_clk_src handling into the GDSC driver
 - Drop interconnect patch (subject for futher research)
 - Remove "pipe" clock from qcom,pcie bindings as it's removed from the
   source code.

Dmitry Baryshkov (11):
  dt-bindings: pci: qcom,pcie: drop unused "pipe" clocks
  dt-bindings: pci: qcom: Document PCIe bindings for SM8450
  clk: qcom: gdsc: add support for clocks tied to the GDSC
  clk: qcom: gcc-sc7280: switch PCIe GDSCs to pipe_clk_gdsc
  clk: qcom: gcc-sm8450: switch PCIe GDSCs to pipe_clk_gdsc
  PCI: qcom: Balance pm_runtime_foo() calls
  PCI: qcom: Remove unnecessary pipe_clk handling
  PCI: qcom: Remove pipe_clk_src reparenting
  PCI: qcom: Remove redundancy between qcom_pcie and qcom_pcie_cfg
  PCI: qcom: Add ddrss_sf_tbu flag
  PCI: qcom: Add SM8450 PCIe support

 .../devicetree/bindings/pci/qcom,pcie.txt     |  20 +-
 drivers/clk/qcom/gcc-sc7280.c                 | 114 ++++-------
 drivers/clk/qcom/gcc-sm8450.c                 | 104 ++++------
 drivers/clk/qcom/gdsc.c                       |  41 ++++
 drivers/clk/qcom/gdsc.h                       |  14 ++
 drivers/pci/controller/dwc/pcie-qcom.c        | 188 ++++++------------
 6 files changed, 219 insertions(+), 262 deletions(-)

-- 
2.34.1

[PATCH v2 01/11] dt-bindings: pci: qcom,pcie: drop unused "pipe" clocks

[Dmitry Baryshkov]

The "pipe" clock is now unused by the PCIe driver. Drop it from the
bindings.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 Documentation/devicetree/bindings/pci/qcom,pcie.txt | 2 --
 1 file changed, 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
index a0ae024c2d0c..da08f0f9de96 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
@@ -154,7 +154,6 @@
 			- "bus_slave"	Slave AXI clock
 			- "slave_q2a"	Slave Q2A clock
 			- "tbu"		PCIe TBU clock
-			- "pipe"	PIPE clock
 
 - clock-names:
 	Usage: required for sc8180x and sm8250
@@ -167,7 +166,6 @@
 			- "slave_q2a"	Slave Q2A clock
 			- "tbu"		PCIe TBU clock
 			- "ddrss_sf_tbu" PCIe SF TBU clock
-			- "pipe"	PIPE clock
 
 - resets:
 	Usage: required
-- 
2.34.1

Re: [PATCH v2 01/11] dt-bindings: pci: qcom,pcie: drop unused "pipe" clocks

[Rob Herring]

On Fri, Feb 04, 2022 at 05:46:35PM +0300, Dmitry Baryshkov wrote:
> The "pipe" clock is now unused by the PCIe driver. Drop it from the
> bindings.

Old kernels expect it, so nak.

> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  Documentation/devicetree/bindings/pci/qcom,pcie.txt | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> index a0ae024c2d0c..da08f0f9de96 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> @@ -154,7 +154,6 @@
>  			- "bus_slave"	Slave AXI clock
>  			- "slave_q2a"	Slave Q2A clock
>  			- "tbu"		PCIe TBU clock
> -			- "pipe"	PIPE clock
>  
>  - clock-names:
>  	Usage: required for sc8180x and sm8250
> @@ -167,7 +166,6 @@
>  			- "slave_q2a"	Slave Q2A clock
>  			- "tbu"		PCIe TBU clock
>  			- "ddrss_sf_tbu" PCIe SF TBU clock
> -			- "pipe"	PIPE clock
>  
>  - resets:
>  	Usage: required
> -- 
> 2.34.1
> 
> 

[PATCH v2 02/11] dt-bindings: pci: qcom: Document PCIe bindings for SM8450

[Dmitry Baryshkov]

Document the PCIe DT bindings for SM8450 SoC. The PCIe IP is similar
to the one used on SM8250, however unlike SM8250, PCIe0 and PCIe1 use
different set of clocks, so two compatible entries are required.

Reviewed-by: Rob Herring <robh@kernel.org>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 .../devicetree/bindings/pci/qcom,pcie.txt      | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
index da08f0f9de96..65a1fa74e4eb 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
@@ -15,6 +15,8 @@
 			- "qcom,pcie-sc8180x" for sc8180x
 			- "qcom,pcie-sdm845" for sdm845
 			- "qcom,pcie-sm8250" for sm8250
+			- "qcom,pcie-sm8450-pcie0" for PCIe0 on sm8450
+			- "qcom,pcie-sm8450-pcie1" for PCIe1 on sm8450
 			- "qcom,pcie-ipq6018" for ipq6018
 
 - reg:
@@ -167,6 +169,20 @@
 			- "tbu"		PCIe TBU clock
 			- "ddrss_sf_tbu" PCIe SF TBU clock
 
+- clock-names:
+	Usage: required for sm8450-pcie0 and sm8450-pcie1
+	Value type: <stringlist>
+	Definition: Should contain the following entries
+			- "aux"         Auxiliary clock
+			- "cfg"         Configuration clock
+			- "bus_master"  Master AXI clock
+			- "bus_slave"   Slave AXI clock
+			- "slave_q2a"   Slave Q2A clock
+			- "tbu"         PCIe TBU clock
+			- "ddrss_sf_tbu" PCIe SF TBU clock
+			- "aggre0"	Aggre NoC PCIe0 AXI clock, only for sm8450-pcie0
+			- "aggre1"	Aggre NoC PCIe1 AXI clock
+
 - resets:
 	Usage: required
 	Value type: <prop-encoded-array>
@@ -244,7 +260,7 @@
 			- "ahb"			AHB reset
 
 - reset-names:
-	Usage: required for sc8180x, sdm845 and sm8250
+	Usage: required for sc8180x, sdm845, sm8250 and sm8450
 	Value type: <stringlist>
 	Definition: Should contain the following entries
 			- "pci"			PCIe core reset
-- 
2.34.1

[PATCH v2 03/11] clk: qcom: gdsc: add support for clocks tied to the GDSC

[Dmitry Baryshkov]

On newer Qualcomm platforms GCC_PCIE_n_PIPE_CLK_SRC should be controlled
together with the PCIE_n_GDSC. The clock should be fed from the TCXO
before switching the GDSC off and can be fed from PCIE_n_PIPE_CLK once
the GDSC is on.

Since commit aa9c0df98c29 ("PCI: qcom: Switch pcie_1_pipe_clk_src after
PHY init in SC7280") PCIe controller driver tries to manage this on it's
own, resulting in the non-optimal code. Furthermore, if the any of the
drivers will have the same requirements, the code would have to be
dupliacted there.

Move handling of such clocks to the GDSC code, providing special GDSC
type.

Cc: Prasad Malisetty <pmaliset@codeaurora.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/clk/qcom/gdsc.c | 41 +++++++++++++++++++++++++++++++++++++++++
 drivers/clk/qcom/gdsc.h | 14 ++++++++++++++
 2 files changed, 55 insertions(+)

diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
index 7e1dd8ccfa38..9913d1b70947 100644
--- a/drivers/clk/qcom/gdsc.c
+++ b/drivers/clk/qcom/gdsc.c
@@ -45,6 +45,7 @@
 #define TIMEOUT_US		500
 
 #define domain_to_gdsc(domain) container_of(domain, struct gdsc, pd)
+#define domain_to_pipe_clk_gdsc(domain) container_of(domain, struct pipe_clk_gdsc, base.pd)
 
 enum gdsc_status {
 	GDSC_OFF,
@@ -549,3 +550,43 @@ int gdsc_gx_do_nothing_enable(struct generic_pm_domain *domain)
 	return 0;
 }
 EXPORT_SYMBOL_GPL(gdsc_gx_do_nothing_enable);
+
+/*
+ * Special operations for GDSCs with attached pipe clocks.
+ * The clock should be parked to safe source (tcxo) before turning off the GDSC
+ * and can be switched on as soon as the GDSC is on.
+ *
+ * We remove respective clock sources from clocks map and handle them manually.
+ */
+int gdsc_pipe_enable(struct generic_pm_domain *domain)
+{
+	struct pipe_clk_gdsc *sc = domain_to_pipe_clk_gdsc(domain);
+	int i, ret;
+
+	ret = gdsc_enable(domain);
+	if (ret)
+		return ret;
+
+	for (i = 0; i< sc->num_clocks; i++)
+		regmap_update_bits(sc->base.regmap, sc->clocks[i].reg,
+				BIT(sc->clocks[i].shift + sc->clocks[i].width) - BIT(sc->clocks[i].shift),
+				sc->clocks[i].on_value << sc->clocks[i].shift);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(gdsc_pipe_enable);
+
+int gdsc_pipe_disable(struct generic_pm_domain *domain)
+{
+	struct pipe_clk_gdsc *sc = domain_to_pipe_clk_gdsc(domain);
+	int i;
+
+	for (i = sc->num_clocks - 1; i >= 0; i--)
+		regmap_update_bits(sc->base.regmap, sc->clocks[i].reg,
+				BIT(sc->clocks[i].shift + sc->clocks[i].width) - BIT(sc->clocks[i].shift),
+				sc->clocks[i].off_value << sc->clocks[i].shift);
+
+	/* In case of an error do not try turning the clocks again. We can not be sure about the GDSC state. */
+	return gdsc_disable(domain);
+}
+EXPORT_SYMBOL_GPL(gdsc_pipe_disable);
diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h
index d7cc4c21a9d4..b1a2f0abe41c 100644
--- a/drivers/clk/qcom/gdsc.h
+++ b/drivers/clk/qcom/gdsc.h
@@ -68,11 +68,25 @@ struct gdsc_desc {
 	size_t num;
 };
 
+struct pipe_clk_gdsc {
+	struct gdsc base;
+	int num_clocks;
+	struct {
+		u32 reg;
+		u32 shift;
+		u32 width;
+		u32 off_value;
+		u32 on_value;
+	} clocks[];
+};
+
 #ifdef CONFIG_QCOM_GDSC
 int gdsc_register(struct gdsc_desc *desc, struct reset_controller_dev *,
 		  struct regmap *);
 void gdsc_unregister(struct gdsc_desc *desc);
 int gdsc_gx_do_nothing_enable(struct generic_pm_domain *domain);
+int gdsc_pipe_enable(struct generic_pm_domain *domain);
+int gdsc_pipe_disable(struct generic_pm_domain *domain);
 #else
 static inline int gdsc_register(struct gdsc_desc *desc,
 				struct reset_controller_dev *rcdev,
-- 
2.34.1

Re: [PATCH v2 03/11] clk: qcom: gdsc: add support for clocks tied to the GDSC

[Bjorn Andersson]

On Fri 04 Feb 08:46 CST 2022, Dmitry Baryshkov wrote:

> On newer Qualcomm platforms GCC_PCIE_n_PIPE_CLK_SRC should be controlled
> together with the PCIE_n_GDSC. The clock should be fed from the TCXO
> before switching the GDSC off and can be fed from PCIE_n_PIPE_CLK once
> the GDSC is on.
> 
> Since commit aa9c0df98c29 ("PCI: qcom: Switch pcie_1_pipe_clk_src after
> PHY init in SC7280") PCIe controller driver tries to manage this on it's
> own, resulting in the non-optimal code. Furthermore, if the any of the
> drivers will have the same requirements, the code would have to be
> dupliacted there.
> 
> Move handling of such clocks to the GDSC code, providing special GDSC
> type.
> 

As discussed on IRC, I'm inclined not to take this, because looks to me
to be the same situation that we have with all GDSCs in SM8350 and
onwards - that some clocks must be parked on a safe parent before the
associated GDSC can be toggled.

Prasad, please advice on what the actual requirements are wrt the
gcc_pipe_clk_src. When does it need to provide a valid signal and when
does it need to be parked?

Regards,
Bjorn

> Cc: Prasad Malisetty <pmaliset@codeaurora.org>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  drivers/clk/qcom/gdsc.c | 41 +++++++++++++++++++++++++++++++++++++++++
>  drivers/clk/qcom/gdsc.h | 14 ++++++++++++++
>  2 files changed, 55 insertions(+)
> 
> diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
> index 7e1dd8ccfa38..9913d1b70947 100644
> --- a/drivers/clk/qcom/gdsc.c
> +++ b/drivers/clk/qcom/gdsc.c
> @@ -45,6 +45,7 @@
>  #define TIMEOUT_US		500
>  
>  #define domain_to_gdsc(domain) container_of(domain, struct gdsc, pd)
> +#define domain_to_pipe_clk_gdsc(domain) container_of(domain, struct pipe_clk_gdsc, base.pd)
>  
>  enum gdsc_status {
>  	GDSC_OFF,
> @@ -549,3 +550,43 @@ int gdsc_gx_do_nothing_enable(struct generic_pm_domain *domain)
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(gdsc_gx_do_nothing_enable);
> +
> +/*
> + * Special operations for GDSCs with attached pipe clocks.
> + * The clock should be parked to safe source (tcxo) before turning off the GDSC
> + * and can be switched on as soon as the GDSC is on.
> + *
> + * We remove respective clock sources from clocks map and handle them manually.
> + */
> +int gdsc_pipe_enable(struct generic_pm_domain *domain)
> +{
> +	struct pipe_clk_gdsc *sc = domain_to_pipe_clk_gdsc(domain);
> +	int i, ret;
> +
> +	ret = gdsc_enable(domain);
> +	if (ret)
> +		return ret;
> +
> +	for (i = 0; i< sc->num_clocks; i++)
> +		regmap_update_bits(sc->base.regmap, sc->clocks[i].reg,
> +				BIT(sc->clocks[i].shift + sc->clocks[i].width) - BIT(sc->clocks[i].shift),
> +				sc->clocks[i].on_value << sc->clocks[i].shift);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(gdsc_pipe_enable);
> +
> +int gdsc_pipe_disable(struct generic_pm_domain *domain)
> +{
> +	struct pipe_clk_gdsc *sc = domain_to_pipe_clk_gdsc(domain);
> +	int i;
> +
> +	for (i = sc->num_clocks - 1; i >= 0; i--)
> +		regmap_update_bits(sc->base.regmap, sc->clocks[i].reg,
> +				BIT(sc->clocks[i].shift + sc->clocks[i].width) - BIT(sc->clocks[i].shift),
> +				sc->clocks[i].off_value << sc->clocks[i].shift);
> +
> +	/* In case of an error do not try turning the clocks again. We can not be sure about the GDSC state. */
> +	return gdsc_disable(domain);
> +}
> +EXPORT_SYMBOL_GPL(gdsc_pipe_disable);
> diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h
> index d7cc4c21a9d4..b1a2f0abe41c 100644
> --- a/drivers/clk/qcom/gdsc.h
> +++ b/drivers/clk/qcom/gdsc.h
> @@ -68,11 +68,25 @@ struct gdsc_desc {
>  	size_t num;
>  };
>  
> +struct pipe_clk_gdsc {
> +	struct gdsc base;
> +	int num_clocks;
> +	struct {
> +		u32 reg;
> +		u32 shift;
> +		u32 width;
> +		u32 off_value;
> +		u32 on_value;
> +	} clocks[];
> +};
> +
>  #ifdef CONFIG_QCOM_GDSC
>  int gdsc_register(struct gdsc_desc *desc, struct reset_controller_dev *,
>  		  struct regmap *);
>  void gdsc_unregister(struct gdsc_desc *desc);
>  int gdsc_gx_do_nothing_enable(struct generic_pm_domain *domain);
> +int gdsc_pipe_enable(struct generic_pm_domain *domain);
> +int gdsc_pipe_disable(struct generic_pm_domain *domain);
>  #else
>  static inline int gdsc_register(struct gdsc_desc *desc,
>  				struct reset_controller_dev *rcdev,
> -- 
> 2.34.1
> 

Re: [PATCH v2 03/11] clk: qcom: gdsc: add support for clocks tied to the GDSC

[Dmitry Baryshkov]

On 05/02/2022 01:05, Bjorn Andersson wrote:
> On Fri 04 Feb 08:46 CST 2022, Dmitry Baryshkov wrote:
> 
>> On newer Qualcomm platforms GCC_PCIE_n_PIPE_CLK_SRC should be controlled
>> together with the PCIE_n_GDSC. The clock should be fed from the TCXO
>> before switching the GDSC off and can be fed from PCIE_n_PIPE_CLK once
>> the GDSC is on.
>>
>> Since commit aa9c0df98c29 ("PCI: qcom: Switch pcie_1_pipe_clk_src after
>> PHY init in SC7280") PCIe controller driver tries to manage this on it's
>> own, resulting in the non-optimal code. Furthermore, if the any of the
>> drivers will have the same requirements, the code would have to be
>> dupliacted there.
>>
>> Move handling of such clocks to the GDSC code, providing special GDSC
>> type.
>>
> 
> As discussed on IRC, I'm inclined not to take this, because looks to me
> to be the same situation that we have with all GDSCs in SM8350 and
> onwards - that some clocks must be parked on a safe parent before the
> associated GDSC can be toggled.
> 
> Prasad, please advice on what the actual requirements are wrt the
> gcc_pipe_clk_src. When does it need to provide a valid signal and when
> does it need to be parked?

Prasad, any comments?

> 
> Regards,
> Bjorn
> 
>> Cc: Prasad Malisetty <pmaliset@codeaurora.org>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> ---
>>   drivers/clk/qcom/gdsc.c | 41 +++++++++++++++++++++++++++++++++++++++++
>>   drivers/clk/qcom/gdsc.h | 14 ++++++++++++++
>>   2 files changed, 55 insertions(+)
>>
>> diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
>> index 7e1dd8ccfa38..9913d1b70947 100644
>> --- a/drivers/clk/qcom/gdsc.c
>> +++ b/drivers/clk/qcom/gdsc.c
>> @@ -45,6 +45,7 @@
>>   #define TIMEOUT_US		500
>>   
>>   #define domain_to_gdsc(domain) container_of(domain, struct gdsc, pd)
>> +#define domain_to_pipe_clk_gdsc(domain) container_of(domain, struct pipe_clk_gdsc, base.pd)
>>   
>>   enum gdsc_status {
>>   	GDSC_OFF,
>> @@ -549,3 +550,43 @@ int gdsc_gx_do_nothing_enable(struct generic_pm_domain *domain)
>>   	return 0;
>>   }
>>   EXPORT_SYMBOL_GPL(gdsc_gx_do_nothing_enable);
>> +
>> +/*
>> + * Special operations for GDSCs with attached pipe clocks.
>> + * The clock should be parked to safe source (tcxo) before turning off the GDSC
>> + * and can be switched on as soon as the GDSC is on.
>> + *
>> + * We remove respective clock sources from clocks map and handle them manually.
>> + */
>> +int gdsc_pipe_enable(struct generic_pm_domain *domain)
>> +{
>> +	struct pipe_clk_gdsc *sc = domain_to_pipe_clk_gdsc(domain);
>> +	int i, ret;
>> +
>> +	ret = gdsc_enable(domain);
>> +	if (ret)
>> +		return ret;
>> +
>> +	for (i = 0; i< sc->num_clocks; i++)
>> +		regmap_update_bits(sc->base.regmap, sc->clocks[i].reg,
>> +				BIT(sc->clocks[i].shift + sc->clocks[i].width) - BIT(sc->clocks[i].shift),
>> +				sc->clocks[i].on_value << sc->clocks[i].shift);
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(gdsc_pipe_enable);
>> +
>> +int gdsc_pipe_disable(struct generic_pm_domain *domain)
>> +{
>> +	struct pipe_clk_gdsc *sc = domain_to_pipe_clk_gdsc(domain);
>> +	int i;
>> +
>> +	for (i = sc->num_clocks - 1; i >= 0; i--)
>> +		regmap_update_bits(sc->base.regmap, sc->clocks[i].reg,
>> +				BIT(sc->clocks[i].shift + sc->clocks[i].width) - BIT(sc->clocks[i].shift),
>> +				sc->clocks[i].off_value << sc->clocks[i].shift);
>> +
>> +	/* In case of an error do not try turning the clocks again. We can not be sure about the GDSC state. */
>> +	return gdsc_disable(domain);
>> +}
>> +EXPORT_SYMBOL_GPL(gdsc_pipe_disable);
>> diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h
>> index d7cc4c21a9d4..b1a2f0abe41c 100644
>> --- a/drivers/clk/qcom/gdsc.h
>> +++ b/drivers/clk/qcom/gdsc.h
>> @@ -68,11 +68,25 @@ struct gdsc_desc {
>>   	size_t num;
>>   };
>>   
>> +struct pipe_clk_gdsc {
>> +	struct gdsc base;
>> +	int num_clocks;
>> +	struct {
>> +		u32 reg;
>> +		u32 shift;
>> +		u32 width;
>> +		u32 off_value;
>> +		u32 on_value;
>> +	} clocks[];
>> +};
>> +
>>   #ifdef CONFIG_QCOM_GDSC
>>   int gdsc_register(struct gdsc_desc *desc, struct reset_controller_dev *,
>>   		  struct regmap *);
>>   void gdsc_unregister(struct gdsc_desc *desc);
>>   int gdsc_gx_do_nothing_enable(struct generic_pm_domain *domain);
>> +int gdsc_pipe_enable(struct generic_pm_domain *domain);
>> +int gdsc_pipe_disable(struct generic_pm_domain *domain);
>>   #else
>>   static inline int gdsc_register(struct gdsc_desc *desc,
>>   				struct reset_controller_dev *rcdev,
>> -- 
>> 2.34.1
>>


-- 
With best wishes
Dmitry

Re: [PATCH v2 03/11] clk: qcom: gdsc: add support for clocks tied to the GDSC

[Dmitry Baryshkov]

On 05/02/2022 01:05, Bjorn Andersson wrote:
> On Fri 04 Feb 08:46 CST 2022, Dmitry Baryshkov wrote:
> 
>> On newer Qualcomm platforms GCC_PCIE_n_PIPE_CLK_SRC should be controlled
>> together with the PCIE_n_GDSC. The clock should be fed from the TCXO
>> before switching the GDSC off and can be fed from PCIE_n_PIPE_CLK once
>> the GDSC is on.
>>
>> Since commit aa9c0df98c29 ("PCI: qcom: Switch pcie_1_pipe_clk_src after
>> PHY init in SC7280") PCIe controller driver tries to manage this on it's
>> own, resulting in the non-optimal code. Furthermore, if the any of the
>> drivers will have the same requirements, the code would have to be
>> dupliacted there.
>>
>> Move handling of such clocks to the GDSC code, providing special GDSC
>> type.
>>
> 
> As discussed on IRC, I'm inclined not to take this, because looks to me
> to be the same situation that we have with all GDSCs in SM8350 and
> onwards - that some clocks must be parked on a safe parent before the
> associated GDSC can be toggled.
> 
> Prasad, please advice on what the actual requirements are wrt the
> gcc_pipe_clk_src. When does it need to provide a valid signal and when
> does it need to be parked?

[Excuse me for the duplicate, Prasad's email was bouncing]

Prasad, any comments?

> 
> Regards,
> Bjorn
> 
>> Cc: Prasad Malisetty <pmaliset@codeaurora.org>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> ---
>>   drivers/clk/qcom/gdsc.c | 41 +++++++++++++++++++++++++++++++++++++++++
>>   drivers/clk/qcom/gdsc.h | 14 ++++++++++++++
>>   2 files changed, 55 insertions(+)
>>
>> diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
>> index 7e1dd8ccfa38..9913d1b70947 100644
>> --- a/drivers/clk/qcom/gdsc.c
>> +++ b/drivers/clk/qcom/gdsc.c
>> @@ -45,6 +45,7 @@
>>   #define TIMEOUT_US		500
>>   
>>   #define domain_to_gdsc(domain) container_of(domain, struct gdsc, pd)
>> +#define domain_to_pipe_clk_gdsc(domain) container_of(domain, struct pipe_clk_gdsc, base.pd)
>>   
>>   enum gdsc_status {
>>   	GDSC_OFF,
>> @@ -549,3 +550,43 @@ int gdsc_gx_do_nothing_enable(struct generic_pm_domain *domain)
>>   	return 0;
>>   }
>>   EXPORT_SYMBOL_GPL(gdsc_gx_do_nothing_enable);
>> +
>> +/*
>> + * Special operations for GDSCs with attached pipe clocks.
>> + * The clock should be parked to safe source (tcxo) before turning off the GDSC
>> + * and can be switched on as soon as the GDSC is on.
>> + *
>> + * We remove respective clock sources from clocks map and handle them manually.
>> + */
>> +int gdsc_pipe_enable(struct generic_pm_domain *domain)
>> +{
>> +	struct pipe_clk_gdsc *sc = domain_to_pipe_clk_gdsc(domain);
>> +	int i, ret;
>> +
>> +	ret = gdsc_enable(domain);
>> +	if (ret)
>> +		return ret;
>> +
>> +	for (i = 0; i< sc->num_clocks; i++)
>> +		regmap_update_bits(sc->base.regmap, sc->clocks[i].reg,
>> +				BIT(sc->clocks[i].shift + sc->clocks[i].width) - BIT(sc->clocks[i].shift),
>> +				sc->clocks[i].on_value << sc->clocks[i].shift);
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(gdsc_pipe_enable);
>> +
>> +int gdsc_pipe_disable(struct generic_pm_domain *domain)
>> +{
>> +	struct pipe_clk_gdsc *sc = domain_to_pipe_clk_gdsc(domain);
>> +	int i;
>> +
>> +	for (i = sc->num_clocks - 1; i >= 0; i--)
>> +		regmap_update_bits(sc->base.regmap, sc->clocks[i].reg,
>> +				BIT(sc->clocks[i].shift + sc->clocks[i].width) - BIT(sc->clocks[i].shift),
>> +				sc->clocks[i].off_value << sc->clocks[i].shift);
>> +
>> +	/* In case of an error do not try turning the clocks again. We can not be sure about the GDSC state. */
>> +	return gdsc_disable(domain);
>> +}
>> +EXPORT_SYMBOL_GPL(gdsc_pipe_disable);
>> diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h
>> index d7cc4c21a9d4..b1a2f0abe41c 100644
>> --- a/drivers/clk/qcom/gdsc.h
>> +++ b/drivers/clk/qcom/gdsc.h
>> @@ -68,11 +68,25 @@ struct gdsc_desc {
>>   	size_t num;
>>   };
>>   
>> +struct pipe_clk_gdsc {
>> +	struct gdsc base;
>> +	int num_clocks;
>> +	struct {
>> +		u32 reg;
>> +		u32 shift;
>> +		u32 width;
>> +		u32 off_value;
>> +		u32 on_value;
>> +	} clocks[];
>> +};
>> +
>>   #ifdef CONFIG_QCOM_GDSC
>>   int gdsc_register(struct gdsc_desc *desc, struct reset_controller_dev *,
>>   		  struct regmap *);
>>   void gdsc_unregister(struct gdsc_desc *desc);
>>   int gdsc_gx_do_nothing_enable(struct generic_pm_domain *domain);
>> +int gdsc_pipe_enable(struct generic_pm_domain *domain);
>> +int gdsc_pipe_disable(struct generic_pm_domain *domain);
>>   #else
>>   static inline int gdsc_register(struct gdsc_desc *desc,
>>   				struct reset_controller_dev *rcdev,
>> -- 
>> 2.34.1
>>


-- 
With best wishes
Dmitry

RE: [PATCH v2 03/11] clk: qcom: gdsc: add support for clocks tied to the GDSC

[Prasad Malisetty (Temp) (QUIC)]

-----Original Message-----
From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 
Sent: Saturday, February 12, 2022 1:23 AM
To: bjorn.andersson@linaro.org; Prasad Malisetty (Temp) (QUIC) <quic_pmaliset@quicinc.com>
Cc: Andy Gross <agross@kernel.org>; Stanimir Varbanov <svarbanov@mm-sol.com>; Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>; Rob Herring <robh+dt@kernel.org>; Krzysztof Wilczy??ski <kw@linux.com>; Michael Turquette <mturquette@baylibre.com>; Stephen Boyd <swboyd@chromium.org>; Bjorn Helgaas <bhelgaas@google.com>; Prasad Malisetty <pmaliset@codeaurora.org>; Vinod Koul <vkoul@kernel.org>; linux-arm-msm@vger.kernel.org; linux-pci@vger.kernel.org; linux-clk@vger.kernel.org; devicetree@vger.kernel.org
Subject: Re: [PATCH v2 03/11] clk: qcom: gdsc: add support for clocks tied to the GDSC

On 05/02/2022 01:05, Bjorn Andersson wrote:
> On Fri 04 Feb 08:46 CST 2022, Dmitry Baryshkov wrote:
> 
>> On newer Qualcomm platforms GCC_PCIE_n_PIPE_CLK_SRC should be 
>> controlled together with the PCIE_n_GDSC. The clock should be fed 
>> from the TCXO before switching the GDSC off and can be fed from 
>> PCIE_n_PIPE_CLK once the GDSC is on.
>>
>> Since commit aa9c0df98c29 ("PCI: qcom: Switch pcie_1_pipe_clk_src 
>> after PHY init in SC7280") PCIe controller driver tries to manage 
>> this on it's own, resulting in the non-optimal code. Furthermore, if 
>> the any of the drivers will have the same requirements, the code 
>> would have to be dupliacted there.
>>
>> Move handling of such clocks to the GDSC code, providing special GDSC 
>> type.
>>
> 
> As discussed on IRC, I'm inclined not to take this, because looks to 
> me to be the same situation that we have with all GDSCs in SM8350 and 
> onwards - that some clocks must be parked on a safe parent before the 
> associated GDSC can be toggled.
> 
> Prasad, please advice on what the actual requirements are wrt the 
> gcc_pipe_clk_src. When does it need to provide a valid signal and when 
> does it need to be parked?

[Excuse me for the duplicate, Prasad's email was bouncing]

Prasad, any comments?

> 
> Regards,
> Bjorn
> 

Hi  Dmitry, 

Greetings !!!

Sorry for the inconvenience,  there was an issue with my mail so I couldn’t receive the updates properly. Now issue is resolved.
I am in discussion with internal team to know more about this. I will update my comments after this discussion.

Thanks
-Prasad 

>> Cc: Prasad Malisetty <pmaliset@codeaurora.org>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> ---
>>   drivers/clk/qcom/gdsc.c | 41 +++++++++++++++++++++++++++++++++++++++++
>>   drivers/clk/qcom/gdsc.h | 14 ++++++++++++++
>>   2 files changed, 55 insertions(+)
>>
>> diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c index 
>> 7e1dd8ccfa38..9913d1b70947 100644
>> --- a/drivers/clk/qcom/gdsc.c
>> +++ b/drivers/clk/qcom/gdsc.c
>> @@ -45,6 +45,7 @@
>>   #define TIMEOUT_US		500
>>   
>>   #define domain_to_gdsc(domain) container_of(domain, struct gdsc, 
>> pd)
>> +#define domain_to_pipe_clk_gdsc(domain) container_of(domain, struct 
>> +pipe_clk_gdsc, base.pd)
>>   
>>   enum gdsc_status {
>>   	GDSC_OFF,
>> @@ -549,3 +550,43 @@ int gdsc_gx_do_nothing_enable(struct generic_pm_domain *domain)
>>   	return 0;
>>   }
>>   EXPORT_SYMBOL_GPL(gdsc_gx_do_nothing_enable);
>> +
>> +/*
>> + * Special operations for GDSCs with attached pipe clocks.
>> + * The clock should be parked to safe source (tcxo) before turning 
>> +off the GDSC
>> + * and can be switched on as soon as the GDSC is on.
>> + *
>> + * We remove respective clock sources from clocks map and handle them manually.
>> + */
>> +int gdsc_pipe_enable(struct generic_pm_domain *domain) {
>> +	struct pipe_clk_gdsc *sc = domain_to_pipe_clk_gdsc(domain);
>> +	int i, ret;
>> +
>> +	ret = gdsc_enable(domain);
>> +	if (ret)
>> +		return ret;
>> +
>> +	for (i = 0; i< sc->num_clocks; i++)
>> +		regmap_update_bits(sc->base.regmap, sc->clocks[i].reg,
>> +				BIT(sc->clocks[i].shift + sc->clocks[i].width) - BIT(sc->clocks[i].shift),
>> +				sc->clocks[i].on_value << sc->clocks[i].shift);
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(gdsc_pipe_enable);
>> +
>> +int gdsc_pipe_disable(struct generic_pm_domain *domain) {
>> +	struct pipe_clk_gdsc *sc = domain_to_pipe_clk_gdsc(domain);
>> +	int i;
>> +
>> +	for (i = sc->num_clocks - 1; i >= 0; i--)
>> +		regmap_update_bits(sc->base.regmap, sc->clocks[i].reg,
>> +				BIT(sc->clocks[i].shift + sc->clocks[i].width) - BIT(sc->clocks[i].shift),
>> +				sc->clocks[i].off_value << sc->clocks[i].shift);
>> +
>> +	/* In case of an error do not try turning the clocks again. We can not be sure about the GDSC state. */
>> +	return gdsc_disable(domain);
>> +}
>> +EXPORT_SYMBOL_GPL(gdsc_pipe_disable);
>> diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h index 
>> d7cc4c21a9d4..b1a2f0abe41c 100644
>> --- a/drivers/clk/qcom/gdsc.h
>> +++ b/drivers/clk/qcom/gdsc.h
>> @@ -68,11 +68,25 @@ struct gdsc_desc {
>>   	size_t num;
>>   };
>>   
>> +struct pipe_clk_gdsc {
>> +	struct gdsc base;
>> +	int num_clocks;
>> +	struct {
>> +		u32 reg;
>> +		u32 shift;
>> +		u32 width;
>> +		u32 off_value;
>> +		u32 on_value;
>> +	} clocks[];
>> +};
>> +
>>   #ifdef CONFIG_QCOM_GDSC
>>   int gdsc_register(struct gdsc_desc *desc, struct reset_controller_dev *,
>>   		  struct regmap *);
>>   void gdsc_unregister(struct gdsc_desc *desc);
>>   int gdsc_gx_do_nothing_enable(struct generic_pm_domain *domain);
>> +int gdsc_pipe_enable(struct generic_pm_domain *domain); int 
>> +gdsc_pipe_disable(struct generic_pm_domain *domain);
>>   #else
>>   static inline int gdsc_register(struct gdsc_desc *desc,
>>   				struct reset_controller_dev *rcdev,
>> --
>> 2.34.1
>>


-- 
With best wishes
Dmitry

Re: [PATCH v2 03/11] clk: qcom: gdsc: add support for clocks tied to the GDSC

[Dmitry Baryshkov]

On Tue, 15 Feb 2022 at 13:24, Prasad Malisetty (Temp) (QUIC)
<quic_pmaliset@quicinc.com> wrote:
>
>
>
> -----Original Message-----
> From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Sent: Saturday, February 12, 2022 1:23 AM
> To: bjorn.andersson@linaro.org; Prasad Malisetty (Temp) (QUIC) <quic_pmaliset@quicinc.com>
> Cc: Andy Gross <agross@kernel.org>; Stanimir Varbanov <svarbanov@mm-sol.com>; Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>; Rob Herring <robh+dt@kernel.org>; Krzysztof Wilczy??ski <kw@linux.com>; Michael Turquette <mturquette@baylibre.com>; Stephen Boyd <swboyd@chromium.org>; Bjorn Helgaas <bhelgaas@google.com>; Prasad Malisetty <pmaliset@codeaurora.org>; Vinod Koul <vkoul@kernel.org>; linux-arm-msm@vger.kernel.org; linux-pci@vger.kernel.org; linux-clk@vger.kernel.org; devicetree@vger.kernel.org
> Subject: Re: [PATCH v2 03/11] clk: qcom: gdsc: add support for clocks tied to the GDSC
>
> On 05/02/2022 01:05, Bjorn Andersson wrote:
> > On Fri 04 Feb 08:46 CST 2022, Dmitry Baryshkov wrote:
> >
> >> On newer Qualcomm platforms GCC_PCIE_n_PIPE_CLK_SRC should be
> >> controlled together with the PCIE_n_GDSC. The clock should be fed
> >> from the TCXO before switching the GDSC off and can be fed from
> >> PCIE_n_PIPE_CLK once the GDSC is on.
> >>
> >> Since commit aa9c0df98c29 ("PCI: qcom: Switch pcie_1_pipe_clk_src
> >> after PHY init in SC7280") PCIe controller driver tries to manage
> >> this on it's own, resulting in the non-optimal code. Furthermore, if
> >> the any of the drivers will have the same requirements, the code
> >> would have to be dupliacted there.
> >>
> >> Move handling of such clocks to the GDSC code, providing special GDSC
> >> type.
> >>
> >
> > As discussed on IRC, I'm inclined not to take this, because looks to
> > me to be the same situation that we have with all GDSCs in SM8350 and
> > onwards - that some clocks must be parked on a safe parent before the
> > associated GDSC can be toggled.
> >
> > Prasad, please advice on what the actual requirements are wrt the
> > gcc_pipe_clk_src. When does it need to provide a valid signal and when
> > does it need to be parked?
>
> [Excuse me for the duplicate, Prasad's email was bouncing]
>
> Prasad, any comments?
>
> >
> > Regards,
> > Bjorn
> >
>
> Hi  Dmitry,
>
> Greetings !!!
>
> Sorry for the inconvenience,  there was an issue with my mail so I couldn’t receive the updates properly. Now issue is resolved.
> I am in discussion with internal team to know more about this. I will update my comments after this discussion.

Prasad, any updates on this topic?

>
> Thanks
> -Prasad
>
> >> Cc: Prasad Malisetty <pmaliset@codeaurora.org>
> >> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> >> ---
> >>   drivers/clk/qcom/gdsc.c | 41 +++++++++++++++++++++++++++++++++++++++++
> >>   drivers/clk/qcom/gdsc.h | 14 ++++++++++++++
> >>   2 files changed, 55 insertions(+)
> >>
> >> diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c index
> >> 7e1dd8ccfa38..9913d1b70947 100644
> >> --- a/drivers/clk/qcom/gdsc.c
> >> +++ b/drivers/clk/qcom/gdsc.c
> >> @@ -45,6 +45,7 @@
> >>   #define TIMEOUT_US         500
> >>
> >>   #define domain_to_gdsc(domain) container_of(domain, struct gdsc,
> >> pd)
> >> +#define domain_to_pipe_clk_gdsc(domain) container_of(domain, struct
> >> +pipe_clk_gdsc, base.pd)
> >>
> >>   enum gdsc_status {
> >>      GDSC_OFF,
> >> @@ -549,3 +550,43 @@ int gdsc_gx_do_nothing_enable(struct generic_pm_domain *domain)
> >>      return 0;
> >>   }
> >>   EXPORT_SYMBOL_GPL(gdsc_gx_do_nothing_enable);
> >> +
> >> +/*
> >> + * Special operations for GDSCs with attached pipe clocks.
> >> + * The clock should be parked to safe source (tcxo) before turning
> >> +off the GDSC
> >> + * and can be switched on as soon as the GDSC is on.
> >> + *
> >> + * We remove respective clock sources from clocks map and handle them manually.
> >> + */
> >> +int gdsc_pipe_enable(struct generic_pm_domain *domain) {
> >> +    struct pipe_clk_gdsc *sc = domain_to_pipe_clk_gdsc(domain);
> >> +    int i, ret;
> >> +
> >> +    ret = gdsc_enable(domain);
> >> +    if (ret)
> >> +            return ret;
> >> +
> >> +    for (i = 0; i< sc->num_clocks; i++)
> >> +            regmap_update_bits(sc->base.regmap, sc->clocks[i].reg,
> >> +                            BIT(sc->clocks[i].shift + sc->clocks[i].width) - BIT(sc->clocks[i].shift),
> >> +                            sc->clocks[i].on_value << sc->clocks[i].shift);
> >> +
> >> +    return 0;
> >> +}
> >> +EXPORT_SYMBOL_GPL(gdsc_pipe_enable);
> >> +
> >> +int gdsc_pipe_disable(struct generic_pm_domain *domain) {
> >> +    struct pipe_clk_gdsc *sc = domain_to_pipe_clk_gdsc(domain);
> >> +    int i;
> >> +
> >> +    for (i = sc->num_clocks - 1; i >= 0; i--)
> >> +            regmap_update_bits(sc->base.regmap, sc->clocks[i].reg,
> >> +                            BIT(sc->clocks[i].shift + sc->clocks[i].width) - BIT(sc->clocks[i].shift),
> >> +                            sc->clocks[i].off_value << sc->clocks[i].shift);
> >> +
> >> +    /* In case of an error do not try turning the clocks again. We can not be sure about the GDSC state. */
> >> +    return gdsc_disable(domain);
> >> +}
> >> +EXPORT_SYMBOL_GPL(gdsc_pipe_disable);
> >> diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h index
> >> d7cc4c21a9d4..b1a2f0abe41c 100644
> >> --- a/drivers/clk/qcom/gdsc.h
> >> +++ b/drivers/clk/qcom/gdsc.h
> >> @@ -68,11 +68,25 @@ struct gdsc_desc {
> >>      size_t num;
> >>   };
> >>
> >> +struct pipe_clk_gdsc {
> >> +    struct gdsc base;
> >> +    int num_clocks;
> >> +    struct {
> >> +            u32 reg;
> >> +            u32 shift;
> >> +            u32 width;
> >> +            u32 off_value;
> >> +            u32 on_value;
> >> +    } clocks[];
> >> +};
> >> +
> >>   #ifdef CONFIG_QCOM_GDSC
> >>   int gdsc_register(struct gdsc_desc *desc, struct reset_controller_dev *,
> >>                struct regmap *);
> >>   void gdsc_unregister(struct gdsc_desc *desc);
> >>   int gdsc_gx_do_nothing_enable(struct generic_pm_domain *domain);
> >> +int gdsc_pipe_enable(struct generic_pm_domain *domain); int
> >> +gdsc_pipe_disable(struct generic_pm_domain *domain);
> >>   #else
> >>   static inline int gdsc_register(struct gdsc_desc *desc,
> >>                              struct reset_controller_dev *rcdev,
> >> --
> >> 2.34.1
> >>
>
>
> --
> With best wishes
> Dmitry



-- 
With best wishes
Dmitry

Re: [PATCH v2 03/11] clk: qcom: gdsc: add support for clocks tied to the GDSC

[Prasad Malisetty]

++ Taniya

Hi Dmitry,

Greetings !!!

I discussed with internal team. setting gcc_pcie_n_pipe_clk src in pcie 
driver doesn't have any relation with gdsc.

But we are making sure that gcc_pcie_n_pipe_clk src is bi_tcxo before 
enabling the clocks and switching to pipe_clk src after PHY is enalbe.

During suspend switching back to bi_tcxo as we enabling the clock as 
part of resume.

  Hi Taniya,

Please provide your inputs.

Thanks

-Prasad
On 2/12/2022 1:22 AM, Dmitry Baryshkov wrote:
> On 05/02/2022 01:05, Bjorn Andersson wrote:
>> On Fri 04 Feb 08:46 CST 2022, Dmitry Baryshkov wrote:
>>
>>> On newer Qualcomm platforms GCC_PCIE_n_PIPE_CLK_SRC should be 
>>> controlled
>>> together with the PCIE_n_GDSC. The clock should be fed from the TCXO
>>> before switching the GDSC off and can be fed from PCIE_n_PIPE_CLK once
>>> the GDSC is on.
>>>
>>> Since commit aa9c0df98c29 ("PCI: qcom: Switch pcie_1_pipe_clk_src after
>>> PHY init in SC7280") PCIe controller driver tries to manage this on 
>>> it's
>>> own, resulting in the non-optimal code. Furthermore, if the any of the
>>> drivers will have the same requirements, the code would have to be
>>> dupliacted there.
>>>
>>> Move handling of such clocks to the GDSC code, providing special GDSC
>>> type.
>>>
>>
>> As discussed on IRC, I'm inclined not to take this, because looks to me
>> to be the same situation that we have with all GDSCs in SM8350 and
>> onwards - that some clocks must be parked on a safe parent before the
>> associated GDSC can be toggled.
>>
>> Prasad, please advice on what the actual requirements are wrt the
>> gcc_pipe_clk_src. When does it need to provide a valid signal and when
>> does it need to be parked?
>
> [Excuse me for the duplicate, Prasad's email was bouncing]
>
> Prasad, any comments?
>
>>
>> Regards,
>> Bjorn
>>
>>> Cc: Prasad Malisetty <pmaliset@codeaurora.org>
>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>> ---
>>>   drivers/clk/qcom/gdsc.c | 41 
>>> +++++++++++++++++++++++++++++++++++++++++
>>>   drivers/clk/qcom/gdsc.h | 14 ++++++++++++++
>>>   2 files changed, 55 insertions(+)
>>>
>>> diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
>>> index 7e1dd8ccfa38..9913d1b70947 100644
>>> --- a/drivers/clk/qcom/gdsc.c
>>> +++ b/drivers/clk/qcom/gdsc.c
>>> @@ -45,6 +45,7 @@
>>>   #define TIMEOUT_US        500
>>>     #define domain_to_gdsc(domain) container_of(domain, struct gdsc, 
>>> pd)
>>> +#define domain_to_pipe_clk_gdsc(domain) container_of(domain, struct 
>>> pipe_clk_gdsc, base.pd)
>>>     enum gdsc_status {
>>>       GDSC_OFF,
>>> @@ -549,3 +550,43 @@ int gdsc_gx_do_nothing_enable(struct 
>>> generic_pm_domain *domain)
>>>       return 0;
>>>   }
>>>   EXPORT_SYMBOL_GPL(gdsc_gx_do_nothing_enable);
>>> +
>>> +/*
>>> + * Special operations for GDSCs with attached pipe clocks.
>>> + * The clock should be parked to safe source (tcxo) before turning 
>>> off the GDSC
>>> + * and can be switched on as soon as the GDSC is on.
>>> + *
>>> + * We remove respective clock sources from clocks map and handle 
>>> them manually.
>>> + */
>>> +int gdsc_pipe_enable(struct generic_pm_domain *domain)
>>> +{
>>> +    struct pipe_clk_gdsc *sc = domain_to_pipe_clk_gdsc(domain);
>>> +    int i, ret;
>>> +
>>> +    ret = gdsc_enable(domain);
>>> +    if (ret)
>>> +        return ret;
>>> +
>>> +    for (i = 0; i< sc->num_clocks; i++)
>>> +        regmap_update_bits(sc->base.regmap, sc->clocks[i].reg,
>>> +                BIT(sc->clocks[i].shift + sc->clocks[i].width) - 
>>> BIT(sc->clocks[i].shift),
>>> +                sc->clocks[i].on_value << sc->clocks[i].shift);
>>> +
>>> +    return 0;
>>> +}
>>> +EXPORT_SYMBOL_GPL(gdsc_pipe_enable);
>>> +
>>> +int gdsc_pipe_disable(struct generic_pm_domain *domain)
>>> +{
>>> +    struct pipe_clk_gdsc *sc = domain_to_pipe_clk_gdsc(domain);
>>> +    int i;
>>> +
>>> +    for (i = sc->num_clocks - 1; i >= 0; i--)
>>> +        regmap_update_bits(sc->base.regmap, sc->clocks[i].reg,
>>> +                BIT(sc->clocks[i].shift + sc->clocks[i].width) - 
>>> BIT(sc->clocks[i].shift),
>>> +                sc->clocks[i].off_value << sc->clocks[i].shift);
>>> +
>>> +    /* In case of an error do not try turning the clocks again. We 
>>> can not be sure about the GDSC state. */
>>> +    return gdsc_disable(domain);
>>> +}
>>> +EXPORT_SYMBOL_GPL(gdsc_pipe_disable);
>>> diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h
>>> index d7cc4c21a9d4..b1a2f0abe41c 100644
>>> --- a/drivers/clk/qcom/gdsc.h
>>> +++ b/drivers/clk/qcom/gdsc.h
>>> @@ -68,11 +68,25 @@ struct gdsc_desc {
>>>       size_t num;
>>>   };
>>>   +struct pipe_clk_gdsc {
>>> +    struct gdsc base;
>>> +    int num_clocks;
>>> +    struct {
>>> +        u32 reg;
>>> +        u32 shift;
>>> +        u32 width;
>>> +        u32 off_value;
>>> +        u32 on_value;
>>> +    } clocks[];
>>> +};
>>> +
>>>   #ifdef CONFIG_QCOM_GDSC
>>>   int gdsc_register(struct gdsc_desc *desc, struct 
>>> reset_controller_dev *,
>>>             struct regmap *);
>>>   void gdsc_unregister(struct gdsc_desc *desc);
>>>   int gdsc_gx_do_nothing_enable(struct generic_pm_domain *domain);
>>> +int gdsc_pipe_enable(struct generic_pm_domain *domain);
>>> +int gdsc_pipe_disable(struct generic_pm_domain *domain);
>>>   #else
>>>   static inline int gdsc_register(struct gdsc_desc *desc,
>>>                   struct reset_controller_dev *rcdev,
>>> -- 
>>> 2.34.1
>>>
>
>

Re: [PATCH v2 03/11] clk: qcom: gdsc: add support for clocks tied to the GDSC

[Dmitry Baryshkov]

Hi,

On Tue, 1 Mar 2022 at 09:42, Prasad Malisetty <quic_pmaliset@quicinc.com> wrote:
> I discussed with internal team. setting gcc_pcie_n_pipe_clk src in pcie
> driver doesn't have any relation with gdsc.
>
> But we are making sure that gcc_pcie_n_pipe_clk src is bi_tcxo before
> enabling the clocks and switching to pipe_clk src after PHY is enalbe.
>
> During suspend switching back to bi_tcxo as we enabling the clock as
> part of resume.

So... I assume that if we implement the enable/disable() ops in a way
similar to clk_rcg2_shared_ops, we can drop all manual handling of
pipe_clk sources.

Bjorn, Taniya WDYT?

>
>   Hi Taniya,
>
> Please provide your inputs.
>
> Thanks
>
> -Prasad
> On 2/12/2022 1:22 AM, Dmitry Baryshkov wrote:
> > On 05/02/2022 01:05, Bjorn Andersson wrote:
> >> On Fri 04 Feb 08:46 CST 2022, Dmitry Baryshkov wrote:
> >>
> >>> On newer Qualcomm platforms GCC_PCIE_n_PIPE_CLK_SRC should be
> >>> controlled
> >>> together with the PCIE_n_GDSC. The clock should be fed from the TCXO
> >>> before switching the GDSC off and can be fed from PCIE_n_PIPE_CLK once
> >>> the GDSC is on.
> >>>
> >>> Since commit aa9c0df98c29 ("PCI: qcom: Switch pcie_1_pipe_clk_src after
> >>> PHY init in SC7280") PCIe controller driver tries to manage this on
> >>> it's
> >>> own, resulting in the non-optimal code. Furthermore, if the any of the
> >>> drivers will have the same requirements, the code would have to be
> >>> dupliacted there.
> >>>
> >>> Move handling of such clocks to the GDSC code, providing special GDSC
> >>> type.
> >>>
> >>
> >> As discussed on IRC, I'm inclined not to take this, because looks to me
> >> to be the same situation that we have with all GDSCs in SM8350 and
> >> onwards - that some clocks must be parked on a safe parent before the
> >> associated GDSC can be toggled.
> >>
> >> Prasad, please advice on what the actual requirements are wrt the
> >> gcc_pipe_clk_src. When does it need to provide a valid signal and when
> >> does it need to be parked?
> >
> > [Excuse me for the duplicate, Prasad's email was bouncing]
> >
> > Prasad, any comments?
> >
> >>
> >> Regards,
> >> Bjorn
> >>
> >>> Cc: Prasad Malisetty <pmaliset@codeaurora.org>
> >>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> >>> ---
> >>>   drivers/clk/qcom/gdsc.c | 41
> >>> +++++++++++++++++++++++++++++++++++++++++
> >>>   drivers/clk/qcom/gdsc.h | 14 ++++++++++++++
> >>>   2 files changed, 55 insertions(+)
> >>>
> >>> diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
> >>> index 7e1dd8ccfa38..9913d1b70947 100644
> >>> --- a/drivers/clk/qcom/gdsc.c
> >>> +++ b/drivers/clk/qcom/gdsc.c
> >>> @@ -45,6 +45,7 @@
> >>>   #define TIMEOUT_US        500
> >>>     #define domain_to_gdsc(domain) container_of(domain, struct gdsc,
> >>> pd)
> >>> +#define domain_to_pipe_clk_gdsc(domain) container_of(domain, struct
> >>> pipe_clk_gdsc, base.pd)
> >>>     enum gdsc_status {
> >>>       GDSC_OFF,
> >>> @@ -549,3 +550,43 @@ int gdsc_gx_do_nothing_enable(struct
> >>> generic_pm_domain *domain)
> >>>       return 0;
> >>>   }
> >>>   EXPORT_SYMBOL_GPL(gdsc_gx_do_nothing_enable);
> >>> +
> >>> +/*
> >>> + * Special operations for GDSCs with attached pipe clocks.
> >>> + * The clock should be parked to safe source (tcxo) before turning
> >>> off the GDSC
> >>> + * and can be switched on as soon as the GDSC is on.
> >>> + *
> >>> + * We remove respective clock sources from clocks map and handle
> >>> them manually.
> >>> + */
> >>> +int gdsc_pipe_enable(struct generic_pm_domain *domain)
> >>> +{
> >>> +    struct pipe_clk_gdsc *sc = domain_to_pipe_clk_gdsc(domain);
> >>> +    int i, ret;
> >>> +
> >>> +    ret = gdsc_enable(domain);
> >>> +    if (ret)
> >>> +        return ret;
> >>> +
> >>> +    for (i = 0; i< sc->num_clocks; i++)
> >>> +        regmap_update_bits(sc->base.regmap, sc->clocks[i].reg,
> >>> +                BIT(sc->clocks[i].shift + sc->clocks[i].width) -
> >>> BIT(sc->clocks[i].shift),
> >>> +                sc->clocks[i].on_value << sc->clocks[i].shift);
> >>> +
> >>> +    return 0;
> >>> +}
> >>> +EXPORT_SYMBOL_GPL(gdsc_pipe_enable);
> >>> +
> >>> +int gdsc_pipe_disable(struct generic_pm_domain *domain)
> >>> +{
> >>> +    struct pipe_clk_gdsc *sc = domain_to_pipe_clk_gdsc(domain);
> >>> +    int i;
> >>> +
> >>> +    for (i = sc->num_clocks - 1; i >= 0; i--)
> >>> +        regmap_update_bits(sc->base.regmap, sc->clocks[i].reg,
> >>> +                BIT(sc->clocks[i].shift + sc->clocks[i].width) -
> >>> BIT(sc->clocks[i].shift),
> >>> +                sc->clocks[i].off_value << sc->clocks[i].shift);
> >>> +
> >>> +    /* In case of an error do not try turning the clocks again. We
> >>> can not be sure about the GDSC state. */
> >>> +    return gdsc_disable(domain);
> >>> +}
> >>> +EXPORT_SYMBOL_GPL(gdsc_pipe_disable);
> >>> diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h
> >>> index d7cc4c21a9d4..b1a2f0abe41c 100644
> >>> --- a/drivers/clk/qcom/gdsc.h
> >>> +++ b/drivers/clk/qcom/gdsc.h
> >>> @@ -68,11 +68,25 @@ struct gdsc_desc {
> >>>       size_t num;
> >>>   };
> >>>   +struct pipe_clk_gdsc {
> >>> +    struct gdsc base;
> >>> +    int num_clocks;
> >>> +    struct {
> >>> +        u32 reg;
> >>> +        u32 shift;
> >>> +        u32 width;
> >>> +        u32 off_value;
> >>> +        u32 on_value;
> >>> +    } clocks[];
> >>> +};
> >>> +
> >>>   #ifdef CONFIG_QCOM_GDSC
> >>>   int gdsc_register(struct gdsc_desc *desc, struct
> >>> reset_controller_dev *,
> >>>             struct regmap *);
> >>>   void gdsc_unregister(struct gdsc_desc *desc);
> >>>   int gdsc_gx_do_nothing_enable(struct generic_pm_domain *domain);
> >>> +int gdsc_pipe_enable(struct generic_pm_domain *domain);
> >>> +int gdsc_pipe_disable(struct generic_pm_domain *domain);
> >>>   #else
> >>>   static inline int gdsc_register(struct gdsc_desc *desc,
> >>>                   struct reset_controller_dev *rcdev,
> >>> --
> >>> 2.34.1
> >>>
> >
> >



-- 
With best wishes
Dmitry

Re: [PATCH v2 03/11] clk: qcom: gdsc: add support for clocks tied to the GDSC

[Bjorn Andersson]

On Mon 28 Feb 22:47 PST 2022, Dmitry Baryshkov wrote:

> Hi,
> 
> On Tue, 1 Mar 2022 at 09:42, Prasad Malisetty <quic_pmaliset@quicinc.com> wrote:
> > I discussed with internal team. setting gcc_pcie_n_pipe_clk src in pcie
> > driver doesn't have any relation with gdsc.
> >
> > But we are making sure that gcc_pcie_n_pipe_clk src is bi_tcxo before
> > enabling the clocks and switching to pipe_clk src after PHY is enalbe.
> >
> > During suspend switching back to bi_tcxo as we enabling the clock as
> > part of resume.
> 
> So... I assume that if we implement the enable/disable() ops in a way
> similar to clk_rcg2_shared_ops, we can drop all manual handling of
> pipe_clk sources.
> 
> Bjorn, Taniya WDYT?
> 

To me it really sounds like the need here is to "park" the pipe clock
source on bi_tcxo while the PHY isn't providing a valid clock signal
into GCC. If so "parking" the clock the same way as the rcg2_shared_ops
seems reasonable in that case.

Also, looking at downstream, the USB pipe clock seems to be handled in a
similar fashion.


But I'm still wondering what the actual requirement for the pipe clock
is. Per your description Prasad, it seems that the PHY doesn't need the
pipe clock coming back from GCC during initialization - and the PCIe
controller driver enables the pipe_clk after powering on the phy.

On platforms prior to there being a mux involved (e.g. SDM845) we have a
branch that is marked BRANCH_HALT_SKIP. But do we have that because we
incorrectly enable the gcc_pipe_clk before we power on the PHY?

I thought we did this because gcc_pipe_clk was part of some feedback
loop when calibrating the PHY PLL, but if sc7280 can feed tcxo that
doesn't make sense. Is the incoming pipe_clk part of the PHY
initialization or not?

Can we move the enablement of gcc_pipe_clk to be done after we bring up
the PHY and thereby drop the BRANCH_HALT_SKIP on these platforms?

Regards,
Bjorn

> >
> >   Hi Taniya,
> >
> > Please provide your inputs.
> >
> > Thanks
> >
> > -Prasad
> > On 2/12/2022 1:22 AM, Dmitry Baryshkov wrote:
> > > On 05/02/2022 01:05, Bjorn Andersson wrote:
> > >> On Fri 04 Feb 08:46 CST 2022, Dmitry Baryshkov wrote:
> > >>
> > >>> On newer Qualcomm platforms GCC_PCIE_n_PIPE_CLK_SRC should be
> > >>> controlled
> > >>> together with the PCIE_n_GDSC. The clock should be fed from the TCXO
> > >>> before switching the GDSC off and can be fed from PCIE_n_PIPE_CLK once
> > >>> the GDSC is on.
> > >>>
> > >>> Since commit aa9c0df98c29 ("PCI: qcom: Switch pcie_1_pipe_clk_src after
> > >>> PHY init in SC7280") PCIe controller driver tries to manage this on
> > >>> it's
> > >>> own, resulting in the non-optimal code. Furthermore, if the any of the
> > >>> drivers will have the same requirements, the code would have to be
> > >>> dupliacted there.
> > >>>
> > >>> Move handling of such clocks to the GDSC code, providing special GDSC
> > >>> type.
> > >>>
> > >>
> > >> As discussed on IRC, I'm inclined not to take this, because looks to me
> > >> to be the same situation that we have with all GDSCs in SM8350 and
> > >> onwards - that some clocks must be parked on a safe parent before the
> > >> associated GDSC can be toggled.
> > >>
> > >> Prasad, please advice on what the actual requirements are wrt the
> > >> gcc_pipe_clk_src. When does it need to provide a valid signal and when
> > >> does it need to be parked?
> > >
> > > [Excuse me for the duplicate, Prasad's email was bouncing]
> > >
> > > Prasad, any comments?
> > >
> > >>
> > >> Regards,
> > >> Bjorn
> > >>
> > >>> Cc: Prasad Malisetty <pmaliset@codeaurora.org>
> > >>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > >>> ---
> > >>>   drivers/clk/qcom/gdsc.c | 41
> > >>> +++++++++++++++++++++++++++++++++++++++++
> > >>>   drivers/clk/qcom/gdsc.h | 14 ++++++++++++++
> > >>>   2 files changed, 55 insertions(+)
> > >>>
> > >>> diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
> > >>> index 7e1dd8ccfa38..9913d1b70947 100644
> > >>> --- a/drivers/clk/qcom/gdsc.c
> > >>> +++ b/drivers/clk/qcom/gdsc.c
> > >>> @@ -45,6 +45,7 @@
> > >>>   #define TIMEOUT_US        500
> > >>>     #define domain_to_gdsc(domain) container_of(domain, struct gdsc,
> > >>> pd)
> > >>> +#define domain_to_pipe_clk_gdsc(domain) container_of(domain, struct
> > >>> pipe_clk_gdsc, base.pd)
> > >>>     enum gdsc_status {
> > >>>       GDSC_OFF,
> > >>> @@ -549,3 +550,43 @@ int gdsc_gx_do_nothing_enable(struct
> > >>> generic_pm_domain *domain)
> > >>>       return 0;
> > >>>   }
> > >>>   EXPORT_SYMBOL_GPL(gdsc_gx_do_nothing_enable);
> > >>> +
> > >>> +/*
> > >>> + * Special operations for GDSCs with attached pipe clocks.
> > >>> + * The clock should be parked to safe source (tcxo) before turning
> > >>> off the GDSC
> > >>> + * and can be switched on as soon as the GDSC is on.
> > >>> + *
> > >>> + * We remove respective clock sources from clocks map and handle
> > >>> them manually.
> > >>> + */
> > >>> +int gdsc_pipe_enable(struct generic_pm_domain *domain)
> > >>> +{
> > >>> +    struct pipe_clk_gdsc *sc = domain_to_pipe_clk_gdsc(domain);
> > >>> +    int i, ret;
> > >>> +
> > >>> +    ret = gdsc_enable(domain);
> > >>> +    if (ret)
> > >>> +        return ret;
> > >>> +
> > >>> +    for (i = 0; i< sc->num_clocks; i++)
> > >>> +        regmap_update_bits(sc->base.regmap, sc->clocks[i].reg,
> > >>> +                BIT(sc->clocks[i].shift + sc->clocks[i].width) -
> > >>> BIT(sc->clocks[i].shift),
> > >>> +                sc->clocks[i].on_value << sc->clocks[i].shift);
> > >>> +
> > >>> +    return 0;
> > >>> +}
> > >>> +EXPORT_SYMBOL_GPL(gdsc_pipe_enable);
> > >>> +
> > >>> +int gdsc_pipe_disable(struct generic_pm_domain *domain)
> > >>> +{
> > >>> +    struct pipe_clk_gdsc *sc = domain_to_pipe_clk_gdsc(domain);
> > >>> +    int i;
> > >>> +
> > >>> +    for (i = sc->num_clocks - 1; i >= 0; i--)
> > >>> +        regmap_update_bits(sc->base.regmap, sc->clocks[i].reg,
> > >>> +                BIT(sc->clocks[i].shift + sc->clocks[i].width) -
> > >>> BIT(sc->clocks[i].shift),
> > >>> +                sc->clocks[i].off_value << sc->clocks[i].shift);
> > >>> +
> > >>> +    /* In case of an error do not try turning the clocks again. We
> > >>> can not be sure about the GDSC state. */
> > >>> +    return gdsc_disable(domain);
> > >>> +}
> > >>> +EXPORT_SYMBOL_GPL(gdsc_pipe_disable);
> > >>> diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h
> > >>> index d7cc4c21a9d4..b1a2f0abe41c 100644
> > >>> --- a/drivers/clk/qcom/gdsc.h
> > >>> +++ b/drivers/clk/qcom/gdsc.h
> > >>> @@ -68,11 +68,25 @@ struct gdsc_desc {
> > >>>       size_t num;
> > >>>   };
> > >>>   +struct pipe_clk_gdsc {
> > >>> +    struct gdsc base;
> > >>> +    int num_clocks;
> > >>> +    struct {
> > >>> +        u32 reg;
> > >>> +        u32 shift;
> > >>> +        u32 width;
> > >>> +        u32 off_value;
> > >>> +        u32 on_value;
> > >>> +    } clocks[];
> > >>> +};
> > >>> +
> > >>>   #ifdef CONFIG_QCOM_GDSC
> > >>>   int gdsc_register(struct gdsc_desc *desc, struct
> > >>> reset_controller_dev *,
> > >>>             struct regmap *);
> > >>>   void gdsc_unregister(struct gdsc_desc *desc);
> > >>>   int gdsc_gx_do_nothing_enable(struct generic_pm_domain *domain);
> > >>> +int gdsc_pipe_enable(struct generic_pm_domain *domain);
> > >>> +int gdsc_pipe_disable(struct generic_pm_domain *domain);
> > >>>   #else
> > >>>   static inline int gdsc_register(struct gdsc_desc *desc,
> > >>>                   struct reset_controller_dev *rcdev,
> > >>> --
> > >>> 2.34.1
> > >>>
> > >
> > >
> 
> 
> 
> -- 
> With best wishes
> Dmitry

[PATCH v2 04/11] clk: qcom: gcc-sc7280: switch PCIe GDSCs to pipe_clk_gdsc

[Dmitry Baryshkov]

Change sc7280's PCIe GDSCs to use new API for managing corresponding
pipe clock sources.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/clk/qcom/gcc-sc7280.c | 114 ++++++++++++----------------------
 1 file changed, 40 insertions(+), 74 deletions(-)

diff --git a/drivers/clk/qcom/gcc-sc7280.c b/drivers/clk/qcom/gcc-sc7280.c
index 423627d49719..e88a2a2416ff 100644
--- a/drivers/clk/qcom/gcc-sc7280.c
+++ b/drivers/clk/qcom/gcc-sc7280.c
@@ -255,26 +255,6 @@ static const struct clk_parent_data gcc_parent_data_5[] = {
 	{ .hw = &gcc_gpll0_out_even.clkr.hw },
 };
 
-static const struct parent_map gcc_parent_map_6[] = {
-	{ P_PCIE_0_PIPE_CLK, 0 },
-	{ P_BI_TCXO, 2 },
-};
-
-static const struct clk_parent_data gcc_parent_data_6[] = {
-	{ .fw_name = "pcie_0_pipe_clk", .name = "pcie_0_pipe_clk" },
-	{ .fw_name = "bi_tcxo" },
-};
-
-static const struct parent_map gcc_parent_map_7[] = {
-	{ P_PCIE_1_PIPE_CLK, 0 },
-	{ P_BI_TCXO, 2 },
-};
-
-static const struct clk_parent_data gcc_parent_data_7[] = {
-	{ .fw_name = "pcie_1_pipe_clk", .name = "pcie_1_pipe_clk" },
-	{ .fw_name = "bi_tcxo" },
-};
-
 static const struct parent_map gcc_parent_map_8[] = {
 	{ P_BI_TCXO, 0 },
 	{ P_GCC_GPLL0_OUT_MAIN, 1 },
@@ -369,36 +349,6 @@ static const struct clk_parent_data gcc_parent_data_15[] = {
 	{ .hw = &gcc_mss_gpll0_main_div_clk_src.clkr.hw },
 };
 
-static struct clk_regmap_mux gcc_pcie_0_pipe_clk_src = {
-	.reg = 0x6b054,
-	.shift = 0,
-	.width = 2,
-	.parent_map = gcc_parent_map_6,
-	.clkr = {
-		.hw.init = &(struct clk_init_data){
-			.name = "gcc_pcie_0_pipe_clk_src",
-			.parent_data = gcc_parent_data_6,
-			.num_parents = ARRAY_SIZE(gcc_parent_data_6),
-			.ops = &clk_regmap_mux_closest_ops,
-		},
-	},
-};
-
-static struct clk_regmap_mux gcc_pcie_1_pipe_clk_src = {
-	.reg = 0x8d054,
-	.shift = 0,
-	.width = 2,
-	.parent_map = gcc_parent_map_7,
-	.clkr = {
-		.hw.init = &(struct clk_init_data){
-			.name = "gcc_pcie_1_pipe_clk_src",
-			.parent_data = gcc_parent_data_7,
-			.num_parents = ARRAY_SIZE(gcc_parent_data_7),
-			.ops = &clk_regmap_mux_closest_ops,
-		},
-	},
-};
-
 static struct clk_regmap_mux gcc_ufs_phy_rx_symbol_0_clk_src = {
 	.reg = 0x77058,
 	.shift = 0,
@@ -1757,10 +1707,6 @@ static struct clk_branch gcc_pcie_0_pipe_clk = {
 		.enable_mask = BIT(4),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_pcie_0_pipe_clk",
-			.parent_hws = (const struct clk_hw*[]){
-				&gcc_pcie_0_pipe_clk_src.clkr.hw,
-			},
-			.num_parents = 1,
 			.flags = CLK_SET_RATE_PARENT,
 			.ops = &clk_branch2_ops,
 		},
@@ -1847,10 +1793,6 @@ static struct clk_branch gcc_pcie_1_pipe_clk = {
 		.enable_mask = BIT(30),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_pcie_1_pipe_clk",
-			.parent_hws = (const struct clk_hw*[]){
-				&gcc_pcie_1_pipe_clk_src.clkr.hw,
-			},
-			.num_parents = 1,
 			.flags = CLK_SET_RATE_PARENT,
 			.ops = &clk_branch2_ops,
 		},
@@ -3113,22 +3055,48 @@ static struct clk_branch gcc_wpss_rscp_clk = {
 	},
 };
 
-static struct gdsc gcc_pcie_0_gdsc = {
-	.gdscr = 0x6b004,
-	.pd = {
-		.name = "gcc_pcie_0_gdsc",
+static struct pipe_clk_gdsc gcc_pcie_0_gdsc = {
+	.base = {
+		.gdscr = 0x6b004,
+		.pd = {
+			.name = "gcc_pcie_0_gdsc",
+		},
+		.pwrsts = PWRSTS_OFF_ON,
+		.flags = VOTABLE,
+	},
+	.num_clocks = 1,
+	.clocks = {
+		{
+			/* gcc_pcie_0_pipe_clk_src */
+			.reg = 0x6b054,
+			.shift = 0,
+			.width = 2,
+			.on_value = 0,
+			.off_value = 2,
+		},
 	},
-	.pwrsts = PWRSTS_OFF_ON,
-	.flags = VOTABLE,
 };
 
-static struct gdsc gcc_pcie_1_gdsc = {
-	.gdscr = 0x8d004,
-	.pd = {
-		.name = "gcc_pcie_1_gdsc",
+static struct pipe_clk_gdsc gcc_pcie_1_gdsc = {
+	.base = {
+		.gdscr = 0x8d004,
+		.pd = {
+			.name = "gcc_pcie_1_gdsc",
+		},
+		.pwrsts = PWRSTS_OFF_ON,
+		.flags = VOTABLE,
+	},
+	.num_clocks = 1,
+	.clocks = {
+		{
+			/* gcc_pcie_1_pipe_clk_src */
+			.reg = 0x8d054,
+			.shift = 0,
+			.width = 2,
+			.on_value = 0,
+			.off_value = 2,
+		},
 	},
-	.pwrsts = PWRSTS_OFF_ON,
-	.flags = VOTABLE,
 };
 
 static struct gdsc gcc_ufs_phy_gdsc = {
@@ -3244,7 +3212,6 @@ static struct clk_regmap *gcc_sc7280_clocks[] = {
 	[GCC_PCIE_0_MSTR_AXI_CLK] = &gcc_pcie_0_mstr_axi_clk.clkr,
 	[GCC_PCIE_0_PHY_RCHNG_CLK_SRC] = &gcc_pcie_0_phy_rchng_clk_src.clkr,
 	[GCC_PCIE_0_PIPE_CLK] = &gcc_pcie_0_pipe_clk.clkr,
-	[GCC_PCIE_0_PIPE_CLK_SRC] = &gcc_pcie_0_pipe_clk_src.clkr,
 	[GCC_PCIE_0_SLV_AXI_CLK] = &gcc_pcie_0_slv_axi_clk.clkr,
 	[GCC_PCIE_0_SLV_Q2A_AXI_CLK] = &gcc_pcie_0_slv_q2a_axi_clk.clkr,
 	[GCC_PCIE_1_AUX_CLK] = &gcc_pcie_1_aux_clk.clkr,
@@ -3253,7 +3220,6 @@ static struct clk_regmap *gcc_sc7280_clocks[] = {
 	[GCC_PCIE_1_MSTR_AXI_CLK] = &gcc_pcie_1_mstr_axi_clk.clkr,
 	[GCC_PCIE_1_PHY_RCHNG_CLK_SRC] = &gcc_pcie_1_phy_rchng_clk_src.clkr,
 	[GCC_PCIE_1_PIPE_CLK] = &gcc_pcie_1_pipe_clk.clkr,
-	[GCC_PCIE_1_PIPE_CLK_SRC] = &gcc_pcie_1_pipe_clk_src.clkr,
 	[GCC_PCIE_1_SLV_AXI_CLK] = &gcc_pcie_1_slv_axi_clk.clkr,
 	[GCC_PCIE_1_SLV_Q2A_AXI_CLK] = &gcc_pcie_1_slv_q2a_axi_clk.clkr,
 	[GCC_PCIE_THROTTLE_CORE_CLK] = &gcc_pcie_throttle_core_clk.clkr,
@@ -3391,8 +3357,8 @@ static struct clk_regmap *gcc_sc7280_clocks[] = {
 };
 
 static struct gdsc *gcc_sc7280_gdscs[] = {
-	[GCC_PCIE_0_GDSC] = &gcc_pcie_0_gdsc,
-	[GCC_PCIE_1_GDSC] = &gcc_pcie_1_gdsc,
+	[GCC_PCIE_0_GDSC] = &gcc_pcie_0_gdsc.base,
+	[GCC_PCIE_1_GDSC] = &gcc_pcie_1_gdsc.base,
 	[GCC_UFS_PHY_GDSC] = &gcc_ufs_phy_gdsc,
 	[GCC_USB30_PRIM_GDSC] = &gcc_usb30_prim_gdsc,
 	[GCC_USB30_SEC_GDSC] = &gcc_usb30_sec_gdsc,
-- 
2.34.1

[PATCH v2 05/11] clk: qcom: gcc-sm8450: switch PCIe GDSCs to pipe_clk_gdsc

[Dmitry Baryshkov]

Change sm8450's PCIe GDSCs to use new API for managing corresponding
pipe clock sources.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/clk/qcom/gcc-sm8450.c | 104 ++++++++++++++--------------------
 1 file changed, 42 insertions(+), 62 deletions(-)

diff --git a/drivers/clk/qcom/gcc-sm8450.c b/drivers/clk/qcom/gcc-sm8450.c
index 593a195467ff..a81456598b28 100644
--- a/drivers/clk/qcom/gcc-sm8450.c
+++ b/drivers/clk/qcom/gcc-sm8450.c
@@ -153,16 +153,6 @@ static const struct clk_parent_data gcc_parent_data_3[] = {
 	{ .fw_name = "bi_tcxo" },
 };
 
-static const struct parent_map gcc_parent_map_4[] = {
-	{ P_PCIE_0_PIPE_CLK, 0 },
-	{ P_BI_TCXO, 2 },
-};
-
-static const struct clk_parent_data gcc_parent_data_4[] = {
-	{ .fw_name = "pcie_0_pipe_clk", },
-	{ .fw_name = "bi_tcxo", },
-};
-
 static const struct parent_map gcc_parent_map_5[] = {
 	{ P_PCIE_1_PHY_AUX_CLK, 0 },
 	{ P_BI_TCXO, 2 },
@@ -239,21 +229,6 @@ static const struct clk_parent_data gcc_parent_data_11[] = {
 	{ .fw_name = "bi_tcxo" },
 };
 
-static struct clk_regmap_mux gcc_pcie_0_pipe_clk_src = {
-	.reg = 0x7b060,
-	.shift = 0,
-	.width = 2,
-	.parent_map = gcc_parent_map_4,
-	.clkr = {
-		.hw.init = &(struct clk_init_data){
-			.name = "gcc_pcie_0_pipe_clk_src",
-			.parent_data = gcc_parent_data_4,
-			.num_parents = ARRAY_SIZE(gcc_parent_data_4),
-			.ops = &clk_regmap_mux_closest_ops,
-		},
-	},
-};
-
 static struct clk_regmap_mux gcc_pcie_1_phy_aux_clk_src = {
 	.reg = 0x9d080,
 	.shift = 0,
@@ -269,21 +244,6 @@ static struct clk_regmap_mux gcc_pcie_1_phy_aux_clk_src = {
 	},
 };
 
-static struct clk_regmap_mux gcc_pcie_1_pipe_clk_src = {
-	.reg = 0x9d064,
-	.shift = 0,
-	.width = 2,
-	.parent_map = gcc_parent_map_6,
-	.clkr = {
-		.hw.init = &(struct clk_init_data){
-			.name = "gcc_pcie_1_pipe_clk_src",
-			.parent_data = gcc_parent_data_6,
-			.num_parents = ARRAY_SIZE(gcc_parent_data_6),
-			.ops = &clk_regmap_mux_closest_ops,
-		},
-	},
-};
-
 static struct clk_regmap_mux gcc_ufs_phy_rx_symbol_0_clk_src = {
 	.reg = 0x87060,
 	.shift = 0,
@@ -1546,10 +1506,6 @@ static struct clk_branch gcc_pcie_0_pipe_clk = {
 		.enable_mask = BIT(4),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_pcie_0_pipe_clk",
-			.parent_data = &(const struct clk_parent_data){
-				.hw = &gcc_pcie_0_pipe_clk_src.clkr.hw,
-			},
-			.num_parents = 1,
 			.flags = CLK_SET_RATE_PARENT,
 			.ops = &clk_branch2_ops,
 		},
@@ -1687,10 +1643,6 @@ static struct clk_branch gcc_pcie_1_pipe_clk = {
 		.enable_mask = BIT(30),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_pcie_1_pipe_clk",
-			.parent_data = &(const struct clk_parent_data){
-				.hw = &gcc_pcie_1_pipe_clk_src.clkr.hw,
-			},
-			.num_parents = 1,
 			.flags = CLK_SET_RATE_PARENT,
 			.ops = &clk_branch2_ops,
 		},
@@ -2952,20 +2904,50 @@ static struct clk_branch gcc_video_axi1_clk = {
 	},
 };
 
-static struct gdsc pcie_0_gdsc = {
-	.gdscr = 0x7b004,
-	.pd = {
-		.name = "pcie_0_gdsc",
+static struct pipe_clk_gdsc pcie_0_gdsc = {
+	.base = {
+		.gdscr = 0x7b004,
+		.pd = {
+			.name = "pcie_0_gdsc",
+			.power_on = gdsc_pipe_enable,
+			.power_off = gdsc_pipe_disable,
+		},
+		.pwrsts = PWRSTS_OFF_ON,
+	},
+	.num_clocks = 1,
+	.clocks = {
+		{
+			/* gcc_pcie_0_pipe_clk_src */
+			.reg = 0x7b060,
+			.shift = 0,
+			.width = 2,
+			.on_value = 0,
+			.off_value = 2,
+		},
 	},
-	.pwrsts = PWRSTS_OFF_ON,
 };
 
-static struct gdsc pcie_1_gdsc = {
-	.gdscr = 0x9d004,
-	.pd = {
-		.name = "pcie_1_gdsc",
+static struct pipe_clk_gdsc pcie_1_gdsc = {
+	.base = {
+		.gdscr = 0x9d004,
+		.pd = {
+			.name = "pcie_1_gdsc",
+			.power_on = gdsc_pipe_enable,
+			.power_off = gdsc_pipe_disable,
+		},
+		.pwrsts = PWRSTS_OFF_ON,
+	},
+	.num_clocks = 1,
+	.clocks = {
+		{
+			/* gcc_pcie_1_pipe_clk_src */
+			.reg = 0x9d064,
+			.shift = 0,
+			.width = 2,
+			.on_value = 0,
+			.off_value = 2,
+		},
 	},
-	.pwrsts = PWRSTS_OFF_ON,
 };
 
 static struct gdsc ufs_phy_gdsc = {
@@ -3022,7 +3004,6 @@ static struct clk_regmap *gcc_sm8450_clocks[] = {
 	[GCC_PCIE_0_PHY_RCHNG_CLK] = &gcc_pcie_0_phy_rchng_clk.clkr,
 	[GCC_PCIE_0_PHY_RCHNG_CLK_SRC] = &gcc_pcie_0_phy_rchng_clk_src.clkr,
 	[GCC_PCIE_0_PIPE_CLK] = &gcc_pcie_0_pipe_clk.clkr,
-	[GCC_PCIE_0_PIPE_CLK_SRC] = &gcc_pcie_0_pipe_clk_src.clkr,
 	[GCC_PCIE_0_SLV_AXI_CLK] = &gcc_pcie_0_slv_axi_clk.clkr,
 	[GCC_PCIE_0_SLV_Q2A_AXI_CLK] = &gcc_pcie_0_slv_q2a_axi_clk.clkr,
 	[GCC_PCIE_1_AUX_CLK] = &gcc_pcie_1_aux_clk.clkr,
@@ -3035,7 +3016,6 @@ static struct clk_regmap *gcc_sm8450_clocks[] = {
 	[GCC_PCIE_1_PHY_RCHNG_CLK] = &gcc_pcie_1_phy_rchng_clk.clkr,
 	[GCC_PCIE_1_PHY_RCHNG_CLK_SRC] = &gcc_pcie_1_phy_rchng_clk_src.clkr,
 	[GCC_PCIE_1_PIPE_CLK] = &gcc_pcie_1_pipe_clk.clkr,
-	[GCC_PCIE_1_PIPE_CLK_SRC] = &gcc_pcie_1_pipe_clk_src.clkr,
 	[GCC_PCIE_1_SLV_AXI_CLK] = &gcc_pcie_1_slv_axi_clk.clkr,
 	[GCC_PCIE_1_SLV_Q2A_AXI_CLK] = &gcc_pcie_1_slv_q2a_axi_clk.clkr,
 	[GCC_PDM2_CLK] = &gcc_pdm2_clk.clkr,
@@ -3216,8 +3196,8 @@ static const struct clk_rcg_dfs_data gcc_dfs_clocks[] = {
 };
 
 static struct gdsc *gcc_sm8450_gdscs[] = {
-	[PCIE_0_GDSC] = &pcie_0_gdsc,
-	[PCIE_1_GDSC] = &pcie_1_gdsc,
+	[PCIE_0_GDSC] = &pcie_0_gdsc.base,
+	[PCIE_1_GDSC] = &pcie_1_gdsc.base,
 	[UFS_PHY_GDSC] = &ufs_phy_gdsc,
 	[USB30_PRIM_GDSC] = &usb30_prim_gdsc,
 };
-- 
2.34.1

Re: [PATCH v2 05/11] clk: qcom: gcc-sm8450: switch PCIe GDSCs to pipe_clk_gdsc

[kernel test robot]

Hi Dmitry,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on helgaas-pci/next]
[also build test WARNING on clk/clk-next v5.17-rc2 next-20220204]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Dmitry-Baryshkov/PCI-qcom-add-support-for-PCIe-on-SM8450-platform/20220204-224841
base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: riscv-randconfig-r036-20220130 (https://download.01.org/0day-ci/archive/20220205/202202050432.j8kshuCH-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project a73e4ce6a59b01f0e37037761c1e6889d539d233)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/0day-ci/linux/commit/5d658c3994422b9324ffa9261c3b5d6288fd1dff
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Dmitry-Baryshkov/PCI-qcom-add-support-for-PCIe-on-SM8450-platform/20220204-224841
        git checkout 5d658c3994422b9324ffa9261c3b5d6288fd1dff
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash drivers/clk/qcom/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/clk/qcom/gcc-sm8450.c:166:32: warning: unused variable 'gcc_parent_map_6' [-Wunused-const-variable]
   static const struct parent_map gcc_parent_map_6[] = {
                                  ^
>> drivers/clk/qcom/gcc-sm8450.c:171:37: warning: unused variable 'gcc_parent_data_6' [-Wunused-const-variable]
   static const struct clk_parent_data gcc_parent_data_6[] = {
                                       ^
   2 warnings generated.


vim +/gcc_parent_map_6 +166 drivers/clk/qcom/gcc-sm8450.c

db0c944ee92b7df Vinod Koul 2021-12-07  165  
db0c944ee92b7df Vinod Koul 2021-12-07 @166  static const struct parent_map gcc_parent_map_6[] = {
db0c944ee92b7df Vinod Koul 2021-12-07  167  	{ P_PCIE_1_PIPE_CLK, 0 },
db0c944ee92b7df Vinod Koul 2021-12-07  168  	{ P_BI_TCXO, 2 },
db0c944ee92b7df Vinod Koul 2021-12-07  169  };
db0c944ee92b7df Vinod Koul 2021-12-07  170  
db0c944ee92b7df Vinod Koul 2021-12-07 @171  static const struct clk_parent_data gcc_parent_data_6[] = {
db0c944ee92b7df Vinod Koul 2021-12-07  172  	{ .fw_name = "pcie_1_pipe_clk" },
db0c944ee92b7df Vinod Koul 2021-12-07  173  	{ .fw_name = "bi_tcxo" },
db0c944ee92b7df Vinod Koul 2021-12-07  174  };
db0c944ee92b7df Vinod Koul 2021-12-07  175  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

Re: [PATCH v2 05/11] clk: qcom: gcc-sm8450: switch PCIe GDSCs to pipe_clk_gdsc

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 3201 bytes --]

Hi Dmitry,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on helgaas-pci/next]
[also build test WARNING on clk/clk-next v5.17-rc2 next-20220204]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Dmitry-Baryshkov/PCI-qcom-add-support-for-PCIe-on-SM8450-platform/20220204-224841
base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: riscv-randconfig-r036-20220130 (https://download.01.org/0day-ci/archive/20220205/202202050432.j8kshuCH-lkp(a)intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project a73e4ce6a59b01f0e37037761c1e6889d539d233)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/0day-ci/linux/commit/5d658c3994422b9324ffa9261c3b5d6288fd1dff
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Dmitry-Baryshkov/PCI-qcom-add-support-for-PCIe-on-SM8450-platform/20220204-224841
        git checkout 5d658c3994422b9324ffa9261c3b5d6288fd1dff
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash drivers/clk/qcom/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/clk/qcom/gcc-sm8450.c:166:32: warning: unused variable 'gcc_parent_map_6' [-Wunused-const-variable]
   static const struct parent_map gcc_parent_map_6[] = {
                                  ^
>> drivers/clk/qcom/gcc-sm8450.c:171:37: warning: unused variable 'gcc_parent_data_6' [-Wunused-const-variable]
   static const struct clk_parent_data gcc_parent_data_6[] = {
                                       ^
   2 warnings generated.


vim +/gcc_parent_map_6 +166 drivers/clk/qcom/gcc-sm8450.c

db0c944ee92b7df Vinod Koul 2021-12-07  165  
db0c944ee92b7df Vinod Koul 2021-12-07 @166  static const struct parent_map gcc_parent_map_6[] = {
db0c944ee92b7df Vinod Koul 2021-12-07  167  	{ P_PCIE_1_PIPE_CLK, 0 },
db0c944ee92b7df Vinod Koul 2021-12-07  168  	{ P_BI_TCXO, 2 },
db0c944ee92b7df Vinod Koul 2021-12-07  169  };
db0c944ee92b7df Vinod Koul 2021-12-07  170  
db0c944ee92b7df Vinod Koul 2021-12-07 @171  static const struct clk_parent_data gcc_parent_data_6[] = {
db0c944ee92b7df Vinod Koul 2021-12-07  172  	{ .fw_name = "pcie_1_pipe_clk" },
db0c944ee92b7df Vinod Koul 2021-12-07  173  	{ .fw_name = "bi_tcxo" },
db0c944ee92b7df Vinod Koul 2021-12-07  174  };
db0c944ee92b7df Vinod Koul 2021-12-07  175  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[PATCH v2 06/11] PCI: qcom: Balance pm_runtime_foo() calls

[Dmitry Baryshkov]

Fix the error path in qcom_pcie_probe(): remove extra calls to
pm_runtime_disable() (which will be called at the end of error path
anyway). Replace a call to pm_runtime_get_sync() with
pm_runtime_resume_and_get() to end up with cleaner code.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index c19cd506ed3f..85c2ad8c551c 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -1549,9 +1549,9 @@ static int qcom_pcie_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	pm_runtime_enable(dev);
-	ret = pm_runtime_get_sync(dev);
+	ret = pm_runtime_resume_and_get(dev);
 	if (ret < 0)
-		goto err_pm_runtime_put;
+		goto err_pm_runtime_disable;
 
 	pci->dev = dev;
 	pci->ops = &dw_pcie_ops;
@@ -1594,7 +1594,6 @@ static int qcom_pcie_probe(struct platform_device *pdev)
 
 	ret = phy_init(pcie->phy);
 	if (ret) {
-		pm_runtime_disable(&pdev->dev);
 		goto err_pm_runtime_put;
 	}
 
@@ -1603,7 +1602,6 @@ static int qcom_pcie_probe(struct platform_device *pdev)
 	ret = dw_pcie_host_init(pp);
 	if (ret) {
 		dev_err(dev, "cannot initialize host\n");
-		pm_runtime_disable(&pdev->dev);
 		goto err_pm_runtime_put;
 	}
 
@@ -1611,6 +1609,7 @@ static int qcom_pcie_probe(struct platform_device *pdev)
 
 err_pm_runtime_put:
 	pm_runtime_put(dev);
+err_pm_runtime_disable:
 	pm_runtime_disable(dev);
 
 	return ret;
-- 
2.34.1

[PATCH v2 07/11] PCI: qcom: Remove unnecessary pipe_clk handling

[Dmitry Baryshkov]

QMP PHY driver already does clk_prepare_enable()/_disable() pipe_clk.
Remove extra calls to enable/disable this clock from the PCIe driver, so
that the PHY driver can manage the clock on its own.

Fixes: aa9c0df98c29 ("PCI: qcom: Switch pcie_1_pipe_clk_src after PHY init in SC7280")
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 53 ++------------------------
 1 file changed, 3 insertions(+), 50 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 85c2ad8c551c..85e84b621dbc 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -128,7 +128,6 @@ struct qcom_pcie_resources_2_3_2 {
 	struct clk *master_clk;
 	struct clk *slave_clk;
 	struct clk *cfg_clk;
-	struct clk *pipe_clk;
 	struct regulator_bulk_data supplies[QCOM_PCIE_2_3_2_MAX_SUPPLY];
 };
 
@@ -165,7 +164,6 @@ struct qcom_pcie_resources_2_7_0 {
 	int num_clks;
 	struct regulator_bulk_data supplies[2];
 	struct reset_control *pci_reset;
-	struct clk *pipe_clk;
 	struct clk *pipe_clk_src;
 	struct clk *phy_pipe_clk;
 	struct clk *ref_clk_src;
@@ -591,11 +589,7 @@ static int qcom_pcie_get_resources_2_3_2(struct qcom_pcie *pcie)
 		return PTR_ERR(res->master_clk);
 
 	res->slave_clk = devm_clk_get(dev, "bus_slave");
-	if (IS_ERR(res->slave_clk))
-		return PTR_ERR(res->slave_clk);
-
-	res->pipe_clk = devm_clk_get(dev, "pipe");
-	return PTR_ERR_OR_ZERO(res->pipe_clk);
+	return PTR_ERR_OR_ZERO(res->slave_clk);
 }
 
 static void qcom_pcie_deinit_2_3_2(struct qcom_pcie *pcie)
@@ -610,13 +604,6 @@ static void qcom_pcie_deinit_2_3_2(struct qcom_pcie *pcie)
 	regulator_bulk_disable(ARRAY_SIZE(res->supplies), res->supplies);
 }
 
-static void qcom_pcie_post_deinit_2_3_2(struct qcom_pcie *pcie)
-{
-	struct qcom_pcie_resources_2_3_2 *res = &pcie->res.v2_3_2;
-
-	clk_disable_unprepare(res->pipe_clk);
-}
-
 static int qcom_pcie_init_2_3_2(struct qcom_pcie *pcie)
 {
 	struct qcom_pcie_resources_2_3_2 *res = &pcie->res.v2_3_2;
@@ -691,22 +678,6 @@ static int qcom_pcie_init_2_3_2(struct qcom_pcie *pcie)
 	return ret;
 }
 
-static int qcom_pcie_post_init_2_3_2(struct qcom_pcie *pcie)
-{
-	struct qcom_pcie_resources_2_3_2 *res = &pcie->res.v2_3_2;
-	struct dw_pcie *pci = pcie->pci;
-	struct device *dev = pci->dev;
-	int ret;
-
-	ret = clk_prepare_enable(res->pipe_clk);
-	if (ret) {
-		dev_err(dev, "cannot prepare/enable pipe clock\n");
-		return ret;
-	}
-
-	return 0;
-}
-
 static int qcom_pcie_get_resources_2_4_0(struct qcom_pcie *pcie)
 {
 	struct qcom_pcie_resources_2_4_0 *res = &pcie->res.v2_4_0;
@@ -1190,8 +1161,7 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie)
 			return PTR_ERR(res->ref_clk_src);
 	}
 
-	res->pipe_clk = devm_clk_get(dev, "pipe");
-	return PTR_ERR_OR_ZERO(res->pipe_clk);
+	return 0;
 }
 
 static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
@@ -1230,12 +1200,6 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
 		goto err_disable_clocks;
 	}
 
-	ret = clk_prepare_enable(res->pipe_clk);
-	if (ret) {
-		dev_err(dev, "cannot prepare/enable pipe clock\n");
-		goto err_disable_clocks;
-	}
-
 	/* configure PCIe to RC mode */
 	writel(DEVICE_TYPE_RC, pcie->parf + PCIE20_PARF_DEVICE_TYPE);
 
@@ -1287,14 +1251,7 @@ static int qcom_pcie_post_init_2_7_0(struct qcom_pcie *pcie)
 	if (pcie->pipe_clk_need_muxing)
 		clk_set_parent(res->pipe_clk_src, res->phy_pipe_clk);
 
-	return clk_prepare_enable(res->pipe_clk);
-}
-
-static void qcom_pcie_post_deinit_2_7_0(struct qcom_pcie *pcie)
-{
-	struct qcom_pcie_resources_2_7_0 *res = &pcie->res.v2_7_0;
-
-	clk_disable_unprepare(res->pipe_clk);
+	return 0;
 }
 
 static int qcom_pcie_link_up(struct dw_pcie *pci)
@@ -1444,9 +1401,7 @@ static const struct qcom_pcie_ops ops_1_0_0 = {
 static const struct qcom_pcie_ops ops_2_3_2 = {
 	.get_resources = qcom_pcie_get_resources_2_3_2,
 	.init = qcom_pcie_init_2_3_2,
-	.post_init = qcom_pcie_post_init_2_3_2,
 	.deinit = qcom_pcie_deinit_2_3_2,
-	.post_deinit = qcom_pcie_post_deinit_2_3_2,
 	.ltssm_enable = qcom_pcie_2_3_2_ltssm_enable,
 };
 
@@ -1473,7 +1428,6 @@ static const struct qcom_pcie_ops ops_2_7_0 = {
 	.deinit = qcom_pcie_deinit_2_7_0,
 	.ltssm_enable = qcom_pcie_2_3_2_ltssm_enable,
 	.post_init = qcom_pcie_post_init_2_7_0,
-	.post_deinit = qcom_pcie_post_deinit_2_7_0,
 };
 
 /* Qcom IP rev.: 1.9.0 */
@@ -1483,7 +1437,6 @@ static const struct qcom_pcie_ops ops_1_9_0 = {
 	.deinit = qcom_pcie_deinit_2_7_0,
 	.ltssm_enable = qcom_pcie_2_3_2_ltssm_enable,
 	.post_init = qcom_pcie_post_init_2_7_0,
-	.post_deinit = qcom_pcie_post_deinit_2_7_0,
 	.config_sid = qcom_pcie_config_sid_sm8250,
 };
 
-- 
2.34.1

[PATCH v2 08/11] PCI: qcom: Remove pipe_clk_src reparenting

[Dmitry Baryshkov]

The SC7280 platform has switched to using GDSC wrapper for setting
pipe_clk_src parent clock. Remove supporting code from the pcie-qcom
driver.

Cc: Prasad Malisetty <pmaliset@codeaurora.org>
Cc: Stephen Boyd <swboyd@chromium.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 38 --------------------------
 1 file changed, 38 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 85e84b621dbc..7df8632a21a8 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -164,9 +164,6 @@ struct qcom_pcie_resources_2_7_0 {
 	int num_clks;
 	struct regulator_bulk_data supplies[2];
 	struct reset_control *pci_reset;
-	struct clk *pipe_clk_src;
-	struct clk *phy_pipe_clk;
-	struct clk *ref_clk_src;
 };
 
 union qcom_pcie_resources {
@@ -192,7 +189,6 @@ struct qcom_pcie_ops {
 
 struct qcom_pcie_cfg {
 	const struct qcom_pcie_ops *ops;
-	unsigned int pipe_clk_need_muxing:1;
 };
 
 struct qcom_pcie {
@@ -203,7 +199,6 @@ struct qcom_pcie {
 	struct phy *phy;
 	struct gpio_desc *reset;
 	const struct qcom_pcie_ops *ops;
-	unsigned int pipe_clk_need_muxing:1;
 };
 
 #define to_qcom_pcie(x)		dev_get_drvdata((x)->dev)
@@ -1147,20 +1142,6 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie)
 	if (ret < 0)
 		return ret;
 
-	if (pcie->pipe_clk_need_muxing) {
-		res->pipe_clk_src = devm_clk_get(dev, "pipe_mux");
-		if (IS_ERR(res->pipe_clk_src))
-			return PTR_ERR(res->pipe_clk_src);
-
-		res->phy_pipe_clk = devm_clk_get(dev, "phy_pipe");
-		if (IS_ERR(res->phy_pipe_clk))
-			return PTR_ERR(res->phy_pipe_clk);
-
-		res->ref_clk_src = devm_clk_get(dev, "ref");
-		if (IS_ERR(res->ref_clk_src))
-			return PTR_ERR(res->ref_clk_src);
-	}
-
 	return 0;
 }
 
@@ -1178,10 +1159,6 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
 		return ret;
 	}
 
-	/* Set TCXO as clock source for pcie_pipe_clk_src */
-	if (pcie->pipe_clk_need_muxing)
-		clk_set_parent(res->pipe_clk_src, res->ref_clk_src);
-
 	ret = clk_bulk_prepare_enable(res->num_clks, res->clks);
 	if (ret < 0)
 		goto err_disable_regulators;
@@ -1243,17 +1220,6 @@ static void qcom_pcie_deinit_2_7_0(struct qcom_pcie *pcie)
 	regulator_bulk_disable(ARRAY_SIZE(res->supplies), res->supplies);
 }
 
-static int qcom_pcie_post_init_2_7_0(struct qcom_pcie *pcie)
-{
-	struct qcom_pcie_resources_2_7_0 *res = &pcie->res.v2_7_0;
-
-	/* Set pipe clock as clock source for pcie_pipe_clk_src */
-	if (pcie->pipe_clk_need_muxing)
-		clk_set_parent(res->pipe_clk_src, res->phy_pipe_clk);
-
-	return 0;
-}
-
 static int qcom_pcie_link_up(struct dw_pcie *pci)
 {
 	u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
@@ -1427,7 +1393,6 @@ static const struct qcom_pcie_ops ops_2_7_0 = {
 	.init = qcom_pcie_init_2_7_0,
 	.deinit = qcom_pcie_deinit_2_7_0,
 	.ltssm_enable = qcom_pcie_2_3_2_ltssm_enable,
-	.post_init = qcom_pcie_post_init_2_7_0,
 };
 
 /* Qcom IP rev.: 1.9.0 */
@@ -1436,7 +1401,6 @@ static const struct qcom_pcie_ops ops_1_9_0 = {
 	.init = qcom_pcie_init_2_7_0,
 	.deinit = qcom_pcie_deinit_2_7_0,
 	.ltssm_enable = qcom_pcie_2_3_2_ltssm_enable,
-	.post_init = qcom_pcie_post_init_2_7_0,
 	.config_sid = qcom_pcie_config_sid_sm8250,
 };
 
@@ -1470,7 +1434,6 @@ static const struct qcom_pcie_cfg sm8250_cfg = {
 
 static const struct qcom_pcie_cfg sc7280_cfg = {
 	.ops = &ops_1_9_0,
-	.pipe_clk_need_muxing = true,
 };
 
 static const struct dw_pcie_ops dw_pcie_ops = {
@@ -1513,7 +1476,6 @@ static int qcom_pcie_probe(struct platform_device *pdev)
 	pcie->pci = pci;
 
 	pcie->ops = pcie_cfg->ops;
-	pcie->pipe_clk_need_muxing = pcie_cfg->pipe_clk_need_muxing;
 
 	pcie->reset = devm_gpiod_get_optional(dev, "perst", GPIOD_OUT_HIGH);
 	if (IS_ERR(pcie->reset)) {
-- 
2.34.1

[PATCH v2 09/11] PCI: qcom: Remove redundancy between qcom_pcie and qcom_pcie_cfg

[Dmitry Baryshkov]

In preparation to adding more flags to configuration data, use pointer
to struct qcom_pcie_cfg directly inside struct qcom_pcie, rather than
duplicating all its fields. This would save us from the boilerplate code
that just copies flag values from one struct to another one.

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 37 ++++++++++++--------------
 1 file changed, 17 insertions(+), 20 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 7df8632a21a8..3a0da577b75d 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -198,7 +198,7 @@ struct qcom_pcie {
 	union qcom_pcie_resources res;
 	struct phy *phy;
 	struct gpio_desc *reset;
-	const struct qcom_pcie_ops *ops;
+	const struct qcom_pcie_cfg *cfg;
 };
 
 #define to_qcom_pcie(x)		dev_get_drvdata((x)->dev)
@@ -222,8 +222,8 @@ static int qcom_pcie_start_link(struct dw_pcie *pci)
 	struct qcom_pcie *pcie = to_qcom_pcie(pci);
 
 	/* Enable Link Training state machine */
-	if (pcie->ops->ltssm_enable)
-		pcie->ops->ltssm_enable(pcie);
+	if (pcie->cfg->ops->ltssm_enable)
+		pcie->cfg->ops->ltssm_enable(pcie);
 
 	return 0;
 }
@@ -1307,7 +1307,7 @@ static int qcom_pcie_host_init(struct pcie_port *pp)
 
 	qcom_ep_reset_assert(pcie);
 
-	ret = pcie->ops->init(pcie);
+	ret = pcie->cfg->ops->init(pcie);
 	if (ret)
 		return ret;
 
@@ -1315,16 +1315,16 @@ static int qcom_pcie_host_init(struct pcie_port *pp)
 	if (ret)
 		goto err_deinit;
 
-	if (pcie->ops->post_init) {
-		ret = pcie->ops->post_init(pcie);
+	if (pcie->cfg->ops->post_init) {
+		ret = pcie->cfg->ops->post_init(pcie);
 		if (ret)
 			goto err_disable_phy;
 	}
 
 	qcom_ep_reset_deassert(pcie);
 
-	if (pcie->ops->config_sid) {
-		ret = pcie->ops->config_sid(pcie);
+	if (pcie->cfg->ops->config_sid) {
+		ret = pcie->cfg->ops->config_sid(pcie);
 		if (ret)
 			goto err;
 	}
@@ -1333,12 +1333,12 @@ static int qcom_pcie_host_init(struct pcie_port *pp)
 
 err:
 	qcom_ep_reset_assert(pcie);
-	if (pcie->ops->post_deinit)
-		pcie->ops->post_deinit(pcie);
+	if (pcie->cfg->ops->post_deinit)
+		pcie->cfg->ops->post_deinit(pcie);
 err_disable_phy:
 	phy_power_off(pcie->phy);
 err_deinit:
-	pcie->ops->deinit(pcie);
+	pcie->cfg->ops->deinit(pcie);
 
 	return ret;
 }
@@ -1447,15 +1447,8 @@ static int qcom_pcie_probe(struct platform_device *pdev)
 	struct pcie_port *pp;
 	struct dw_pcie *pci;
 	struct qcom_pcie *pcie;
-	const struct qcom_pcie_cfg *pcie_cfg;
 	int ret;
 
-	pcie_cfg = of_device_get_match_data(dev);
-	if (!pcie_cfg || !pcie_cfg->ops) {
-		dev_err(dev, "Invalid platform data\n");
-		return -EINVAL;
-	}
-
 	pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
 	if (!pcie)
 		return -ENOMEM;
@@ -1475,7 +1468,11 @@ static int qcom_pcie_probe(struct platform_device *pdev)
 
 	pcie->pci = pci;
 
-	pcie->ops = pcie_cfg->ops;
+	pcie->cfg = of_device_get_match_data(dev);
+	if (!pcie->cfg || !pcie->cfg->ops) {
+		dev_err(dev, "Invalid platform data\n");
+		return -EINVAL;
+	}
 
 	pcie->reset = devm_gpiod_get_optional(dev, "perst", GPIOD_OUT_HIGH);
 	if (IS_ERR(pcie->reset)) {
@@ -1501,7 +1498,7 @@ static int qcom_pcie_probe(struct platform_device *pdev)
 		goto err_pm_runtime_put;
 	}
 
-	ret = pcie->ops->get_resources(pcie);
+	ret = pcie->cfg->ops->get_resources(pcie);
 	if (ret)
 		goto err_pm_runtime_put;
 
-- 
2.34.1

[PATCH v2 10/11] PCI: qcom: Add ddrss_sf_tbu flag

[Dmitry Baryshkov]

Qualcomm PCIe driver uses compatible string to check if the ddrss_sf_tbu
clock should be used. Since sc7280 support has added flags, switch to
the new mechanism to check if this clock should be used.

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 3a0da577b75d..6034a933814d 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -189,6 +189,7 @@ struct qcom_pcie_ops {
 
 struct qcom_pcie_cfg {
 	const struct qcom_pcie_ops *ops;
+	bool has_ddrss_sf_tbu_clk;
 };
 
 struct qcom_pcie {
@@ -1131,7 +1132,7 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie)
 	res->clks[3].id = "bus_slave";
 	res->clks[4].id = "slave_q2a";
 	res->clks[5].id = "tbu";
-	if (of_device_is_compatible(dev->of_node, "qcom,pcie-sm8250")) {
+	if (pcie->cfg->has_ddrss_sf_tbu_clk) {
 		res->clks[6].id = "ddrss_sf_tbu";
 		res->num_clks = 7;
 	} else {
@@ -1430,6 +1431,7 @@ static const struct qcom_pcie_cfg sdm845_cfg = {
 
 static const struct qcom_pcie_cfg sm8250_cfg = {
 	.ops = &ops_1_9_0,
+	.has_ddrss_sf_tbu_clk = true,
 };
 
 static const struct qcom_pcie_cfg sc7280_cfg = {
-- 
2.34.1

[PATCH v2 11/11] PCI: qcom: Add SM8450 PCIe support

[Dmitry Baryshkov]

On SM8450 platform PCIe hosts do not use all the clocks (and add several
additional clocks), so expand the driver to handle these requirements.

PCIe0 and PCIe1 hosts use different sets of clocks, so separate entries
are required.

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 55 ++++++++++++++++++++------
 1 file changed, 42 insertions(+), 13 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 6034a933814d..174a650ffbbb 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -160,7 +160,7 @@ struct qcom_pcie_resources_2_3_3 {
 
 /* 6 clocks typically, 7 for sm8250 */
 struct qcom_pcie_resources_2_7_0 {
-	struct clk_bulk_data clks[7];
+	struct clk_bulk_data clks[9];
 	int num_clks;
 	struct regulator_bulk_data supplies[2];
 	struct reset_control *pci_reset;
@@ -189,7 +189,10 @@ struct qcom_pcie_ops {
 
 struct qcom_pcie_cfg {
 	const struct qcom_pcie_ops *ops;
+	bool has_tbu_clk;
 	bool has_ddrss_sf_tbu_clk;
+	bool has_aggre0_clk;
+	bool has_aggre1_clk;
 };
 
 struct qcom_pcie {
@@ -1113,6 +1116,7 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie)
 	struct qcom_pcie_resources_2_7_0 *res = &pcie->res.v2_7_0;
 	struct dw_pcie *pci = pcie->pci;
 	struct device *dev = pci->dev;
+	unsigned int idx;
 	int ret;
 
 	res->pci_reset = devm_reset_control_get_exclusive(dev, "pci");
@@ -1126,18 +1130,22 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie)
 	if (ret)
 		return ret;
 
-	res->clks[0].id = "aux";
-	res->clks[1].id = "cfg";
-	res->clks[2].id = "bus_master";
-	res->clks[3].id = "bus_slave";
-	res->clks[4].id = "slave_q2a";
-	res->clks[5].id = "tbu";
-	if (pcie->cfg->has_ddrss_sf_tbu_clk) {
-		res->clks[6].id = "ddrss_sf_tbu";
-		res->num_clks = 7;
-	} else {
-		res->num_clks = 6;
-	}
+	idx = 0;
+	res->clks[idx++].id = "aux";
+	res->clks[idx++].id = "cfg";
+	res->clks[idx++].id = "bus_master";
+	res->clks[idx++].id = "bus_slave";
+	res->clks[idx++].id = "slave_q2a";
+	if (pcie->cfg->has_tbu_clk)
+		res->clks[idx++].id = "tbu";
+	if (pcie->cfg->has_ddrss_sf_tbu_clk)
+		res->clks[idx++].id = "ddrss_sf_tbu";
+	if (pcie->cfg->has_aggre0_clk)
+		res->clks[idx++].id = "aggre0";
+	if (pcie->cfg->has_aggre1_clk)
+		res->clks[idx++].id = "aggre1";
+
+	res->num_clks = idx;
 
 	ret = devm_clk_bulk_get(dev, res->num_clks, res->clks);
 	if (ret < 0)
@@ -1178,6 +1186,9 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
 		goto err_disable_clocks;
 	}
 
+	/* Wait for reset to complete, required on SM8450 */
+	usleep_range(1000, 1500);
+
 	/* configure PCIe to RC mode */
 	writel(DEVICE_TYPE_RC, pcie->parf + PCIE20_PARF_DEVICE_TYPE);
 
@@ -1427,15 +1438,31 @@ static const struct qcom_pcie_cfg ipq4019_cfg = {
 
 static const struct qcom_pcie_cfg sdm845_cfg = {
 	.ops = &ops_2_7_0,
+	.has_tbu_clk = true,
 };
 
 static const struct qcom_pcie_cfg sm8250_cfg = {
+	.ops = &ops_1_9_0,
+	.has_tbu_clk = true,
+	.has_ddrss_sf_tbu_clk = true,
+};
+
+static const struct qcom_pcie_cfg sm8450_pcie0_cfg = {
+	.ops = &ops_1_9_0,
+	.has_ddrss_sf_tbu_clk = true,
+	.has_aggre0_clk = true,
+	.has_aggre1_clk = true,
+};
+
+static const struct qcom_pcie_cfg sm8450_pcie1_cfg = {
 	.ops = &ops_1_9_0,
 	.has_ddrss_sf_tbu_clk = true,
+	.has_aggre1_clk = true,
 };
 
 static const struct qcom_pcie_cfg sc7280_cfg = {
 	.ops = &ops_1_9_0,
+	.has_tbu_clk = true,
 };
 
 static const struct dw_pcie_ops dw_pcie_ops = {
@@ -1541,6 +1568,8 @@ static const struct of_device_id qcom_pcie_match[] = {
 	{ .compatible = "qcom,pcie-sdm845", .data = &sdm845_cfg },
 	{ .compatible = "qcom,pcie-sm8250", .data = &sm8250_cfg },
 	{ .compatible = "qcom,pcie-sc8180x", .data = &sm8250_cfg },
+	{ .compatible = "qcom,pcie-sm8450-pcie0", .data = &sm8450_pcie0_cfg },
+	{ .compatible = "qcom,pcie-sm8450-pcie1", .data = &sm8450_pcie1_cfg },
 	{ .compatible = "qcom,pcie-sc7280", .data = &sc7280_cfg },
 	{ }
 };
-- 
2.34.1