diff mbox series

[v7,10/12] PCI: qcom: Add SM8550 PCIe support

Message ID 20230203081807.2248625-11-abel.vesa@linaro.org (mailing list archive)
State Superseded
Headers show
Series sm8550: Add PCIe HC and PHY support | expand

Commit Message

Abel Vesa Feb. 3, 2023, 8:18 a.m. UTC
Add compatible for both PCIe found on SM8550.
Also add the cnoc_pcie_sf_axi clock needed by the SM8550.

Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
---

The v6 of this patchset is:
https://lore.kernel.org/all/20230202123902.3831491-11-abel.vesa@linaro.org/

Changes since v6:
 * none

Changes since v5:
 * none

Changes since v4:
 * added Mani's R-b tag

Changes since v3:
 * renamed cnoc_pcie_sf_axi to cnoc_sf_axi

Changes since v2:
 * none

Changes since v1:
 * changed the subject line prefix for the patch to match the history,
   like Bjorn Helgaas suggested.
 * added Konrad's R-b tag

 drivers/pci/controller/dwc/pcie-qcom.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

Comments

Johan Hovold Feb. 3, 2023, 9:49 a.m. UTC | #1
On Fri, Feb 03, 2023 at 10:18:05AM +0200, Abel Vesa wrote:
> Add compatible for both PCIe found on SM8550.
> Also add the cnoc_pcie_sf_axi clock needed by the SM8550.

nit: You're now also adding 'noc_aggr'

> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
> ---
> 
> The v6 of this patchset is:
> https://lore.kernel.org/all/20230202123902.3831491-11-abel.vesa@linaro.org/
> 
> Changes since v6:
>  * none
> 
> Changes since v5:
>  * none
> 
> Changes since v4:
>  * added Mani's R-b tag
> 
> Changes since v3:
>  * renamed cnoc_pcie_sf_axi to cnoc_sf_axi
> 
> Changes since v2:
>  * none
> 
> Changes since v1:
>  * changed the subject line prefix for the patch to match the history,
>    like Bjorn Helgaas suggested.
>  * added Konrad's R-b tag
> 
>  drivers/pci/controller/dwc/pcie-qcom.c | 25 ++++++++++++++-----------
>  1 file changed, 14 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index a232b04af048..6a70c9c6f98d 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -182,10 +182,10 @@ 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[12];
> +	struct clk_bulk_data clks[14];
>  	int num_clks;
>  	struct regulator_bulk_data supplies[2];
> -	struct reset_control *pci_reset;
> +	struct reset_control *rst;

Please name this one 'reset' or 'resets' (e.g. to avoid hard to parse
things like res->rst below).

>  };
>  
>  struct qcom_pcie_resources_2_9_0 {
> @@ -1177,9 +1177,9 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie)
>  	unsigned int idx;
>  	int ret;
>  
> -	res->pci_reset = devm_reset_control_get_exclusive(dev, "pci");
> -	if (IS_ERR(res->pci_reset))
> -		return PTR_ERR(res->pci_reset);
> +	res->rst = devm_reset_control_array_get_exclusive(dev);
> +	if (IS_ERR(res->rst))
> +		return PTR_ERR(res->rst);

So the reset array implementation apparently both asserts and deasserts
the resets in the order specified in DT (i.e. does not deassert in
reverse order).

Is that ok also for the new "pci" and "link_down" resets?

>  	res->supplies[0].supply = "vdda";
>  	res->supplies[1].supply = "vddpe-3v3";
> @@ -1205,9 +1205,11 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie)
>  	res->clks[idx++].id = "ddrss_sf_tbu";
>  	res->clks[idx++].id = "aggre0";
>  	res->clks[idx++].id = "aggre1";
> +	res->clks[idx++].id = "noc_aggr";
>  	res->clks[idx++].id = "noc_aggr_4";
>  	res->clks[idx++].id = "noc_aggr_south_sf";
>  	res->clks[idx++].id = "cnoc_qx";
> +	res->clks[idx++].id = "cnoc_sf_axi";
>  
>  	num_opt_clks = idx - num_clks;
>  	res->num_clks = idx;
> @@ -1237,17 +1239,17 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
>  	if (ret < 0)
>  		goto err_disable_regulators;
>  
> -	ret = reset_control_assert(res->pci_reset);
> -	if (ret < 0) {
> -		dev_err(dev, "cannot assert pci reset\n");
> +	ret = reset_control_assert(res->rst);
> +	if (ret) {
> +		dev_err(dev, "reset assert failed (%d)\n", ret);
>  		goto err_disable_clocks;
>  	}
>  
>  	usleep_range(1000, 1500);
>  
> -	ret = reset_control_deassert(res->pci_reset);
> -	if (ret < 0) {
> -		dev_err(dev, "cannot deassert pci reset\n");
> +	ret = reset_control_deassert(res->rst);
> +	if (ret) {
> +		dev_err(dev, "reset deassert failed (%d)\n", ret);
>  		goto err_disable_clocks;
>  	}

Johan
Abel Vesa Feb. 6, 2023, 3:11 p.m. UTC | #2
On 23-02-03 10:49:24, Johan Hovold wrote:
> On Fri, Feb 03, 2023 at 10:18:05AM +0200, Abel Vesa wrote:
> > Add compatible for both PCIe found on SM8550.
> > Also add the cnoc_pcie_sf_axi clock needed by the SM8550.
> 
> nit: You're now also adding 'noc_aggr'
> 
> > Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> > Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> > Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
> > ---
> > 
> > The v6 of this patchset is:
> > https://lore.kernel.org/all/20230202123902.3831491-11-abel.vesa@linaro.org/
> > 
> > Changes since v6:
> >  * none
> > 
> > Changes since v5:
> >  * none
> > 
> > Changes since v4:
> >  * added Mani's R-b tag
> > 
> > Changes since v3:
> >  * renamed cnoc_pcie_sf_axi to cnoc_sf_axi
> > 
> > Changes since v2:
> >  * none
> > 
> > Changes since v1:
> >  * changed the subject line prefix for the patch to match the history,
> >    like Bjorn Helgaas suggested.
> >  * added Konrad's R-b tag
> > 
> >  drivers/pci/controller/dwc/pcie-qcom.c | 25 ++++++++++++++-----------
> >  1 file changed, 14 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> > index a232b04af048..6a70c9c6f98d 100644
> > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > @@ -182,10 +182,10 @@ 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[12];
> > +	struct clk_bulk_data clks[14];
> >  	int num_clks;
> >  	struct regulator_bulk_data supplies[2];
> > -	struct reset_control *pci_reset;
> > +	struct reset_control *rst;
> 
> Please name this one 'reset' or 'resets' (e.g. to avoid hard to parse
> things like res->rst below).

Well, it would then be inconsitent with 2_3_3 and 2_9_0, which both use
rst.

> 
> >  };
> >  
> >  struct qcom_pcie_resources_2_9_0 {
> > @@ -1177,9 +1177,9 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie)
> >  	unsigned int idx;
> >  	int ret;
> >  
> > -	res->pci_reset = devm_reset_control_get_exclusive(dev, "pci");
> > -	if (IS_ERR(res->pci_reset))
> > -		return PTR_ERR(res->pci_reset);
> > +	res->rst = devm_reset_control_array_get_exclusive(dev);
> > +	if (IS_ERR(res->rst))
> > +		return PTR_ERR(res->rst);
> 
> So the reset array implementation apparently both asserts and deasserts
> the resets in the order specified in DT (i.e. does not deassert in
> reverse order).
> 
> Is that ok also for the new "pci" and "link_down" resets?

According to the HPG, yes, this is perfectly fine. It specifically says
to assert the pcie reset and then continues saying to assert the
link_down reset.

> 
> >  	res->supplies[0].supply = "vdda";
> >  	res->supplies[1].supply = "vddpe-3v3";
> > @@ -1205,9 +1205,11 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie)
> >  	res->clks[idx++].id = "ddrss_sf_tbu";
> >  	res->clks[idx++].id = "aggre0";
> >  	res->clks[idx++].id = "aggre1";
> > +	res->clks[idx++].id = "noc_aggr";
> >  	res->clks[idx++].id = "noc_aggr_4";
> >  	res->clks[idx++].id = "noc_aggr_south_sf";
> >  	res->clks[idx++].id = "cnoc_qx";
> > +	res->clks[idx++].id = "cnoc_sf_axi";
> >  
> >  	num_opt_clks = idx - num_clks;
> >  	res->num_clks = idx;
> > @@ -1237,17 +1239,17 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
> >  	if (ret < 0)
> >  		goto err_disable_regulators;
> >  
> > -	ret = reset_control_assert(res->pci_reset);
> > -	if (ret < 0) {
> > -		dev_err(dev, "cannot assert pci reset\n");
> > +	ret = reset_control_assert(res->rst);
> > +	if (ret) {
> > +		dev_err(dev, "reset assert failed (%d)\n", ret);
> >  		goto err_disable_clocks;
> >  	}
> >  
> >  	usleep_range(1000, 1500);
> >  
> > -	ret = reset_control_deassert(res->pci_reset);
> > -	if (ret < 0) {
> > -		dev_err(dev, "cannot deassert pci reset\n");
> > +	ret = reset_control_deassert(res->rst);
> > +	if (ret) {
> > +		dev_err(dev, "reset deassert failed (%d)\n", ret);
> >  		goto err_disable_clocks;
> >  	}
> 
> Johan
Johan Hovold Feb. 8, 2023, 4:40 p.m. UTC | #3
On Mon, Feb 06, 2023 at 05:11:01PM +0200, Abel Vesa wrote:
> On 23-02-03 10:49:24, Johan Hovold wrote:
> > On Fri, Feb 03, 2023 at 10:18:05AM +0200, Abel Vesa wrote:
> > > Add compatible for both PCIe found on SM8550.
> > > Also add the cnoc_pcie_sf_axi clock needed by the SM8550.
> > 
> > nit: You're now also adding 'noc_aggr'
> > 
> > > Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> > > Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> > > Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
> > > ---

> > > @@ -182,10 +182,10 @@ 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[12];
> > > +	struct clk_bulk_data clks[14];
> > >  	int num_clks;
> > >  	struct regulator_bulk_data supplies[2];
> > > -	struct reset_control *pci_reset;
> > > +	struct reset_control *rst;
> > 
> > Please name this one 'reset' or 'resets' (e.g. to avoid hard to parse
> > things like res->rst below).
> 
> Well, it would then be inconsitent with 2_3_3 and 2_9_0, which both use
> rst.

Yeah, I saw that. Fortunately these resources are completely
independent, but whatever.
 
> > >  };
> > >  
> > >  struct qcom_pcie_resources_2_9_0 {
> > > @@ -1177,9 +1177,9 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie)
> > >  	unsigned int idx;
> > >  	int ret;
> > >  
> > > -	res->pci_reset = devm_reset_control_get_exclusive(dev, "pci");
> > > -	if (IS_ERR(res->pci_reset))
> > > -		return PTR_ERR(res->pci_reset);
> > > +	res->rst = devm_reset_control_array_get_exclusive(dev);
> > > +	if (IS_ERR(res->rst))
> > > +		return PTR_ERR(res->rst);
> > 
> > So the reset array implementation apparently both asserts and deasserts
> > the resets in the order specified in DT (i.e. does not deassert in
> > reverse order).
> > 
> > Is that ok also for the new "pci" and "link_down" resets?
> 
> According to the HPG, yes, this is perfectly fine. It specifically says
> to assert the pcie reset and then continues saying to assert the
> link_down reset.

Ok, but that doesn't really say anything about whether it's ok to
*deassert* them in the same order, which was what I asked about.

Johan
Abel Vesa Feb. 8, 2023, 5:10 p.m. UTC | #4
On 23-02-08 17:40:03, Johan Hovold wrote:
> On Mon, Feb 06, 2023 at 05:11:01PM +0200, Abel Vesa wrote:
> > On 23-02-03 10:49:24, Johan Hovold wrote:
> > > On Fri, Feb 03, 2023 at 10:18:05AM +0200, Abel Vesa wrote:
> > > > Add compatible for both PCIe found on SM8550.
> > > > Also add the cnoc_pcie_sf_axi clock needed by the SM8550.
> > > 
> > > nit: You're now also adding 'noc_aggr'
> > > 
> > > > Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> > > > Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> > > > Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
> > > > ---
> 
> > > > @@ -182,10 +182,10 @@ 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[12];
> > > > +	struct clk_bulk_data clks[14];
> > > >  	int num_clks;
> > > >  	struct regulator_bulk_data supplies[2];
> > > > -	struct reset_control *pci_reset;
> > > > +	struct reset_control *rst;
> > > 
> > > Please name this one 'reset' or 'resets' (e.g. to avoid hard to parse
> > > things like res->rst below).
> > 
> > Well, it would then be inconsitent with 2_3_3 and 2_9_0, which both use
> > rst.
> 
> Yeah, I saw that. Fortunately these resources are completely
> independent, but whatever.

Will do it in the next version then.

>  
> > > >  };
> > > >  
> > > >  struct qcom_pcie_resources_2_9_0 {
> > > > @@ -1177,9 +1177,9 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie)
> > > >  	unsigned int idx;
> > > >  	int ret;
> > > >  
> > > > -	res->pci_reset = devm_reset_control_get_exclusive(dev, "pci");
> > > > -	if (IS_ERR(res->pci_reset))
> > > > -		return PTR_ERR(res->pci_reset);
> > > > +	res->rst = devm_reset_control_array_get_exclusive(dev);
> > > > +	if (IS_ERR(res->rst))
> > > > +		return PTR_ERR(res->rst);
> > > 
> > > So the reset array implementation apparently both asserts and deasserts
> > > the resets in the order specified in DT (i.e. does not deassert in
> > > reverse order).
> > > 
> > > Is that ok also for the new "pci" and "link_down" resets?
> > 
> > According to the HPG, yes, this is perfectly fine. It specifically says
> > to assert the pcie reset and then continues saying to assert the
> > link_down reset.
> 
> Ok, but that doesn't really say anything about whether it's ok to
> *deassert* them in the same order, which was what I asked about.

Actually, what I wanted to say is that the HPG says something like this:

"assert pcie reset, then assert link_down"

and then at the end it literaly repeats the same phrase.





> 
> Johan
Abel Vesa Feb. 8, 2023, 5:11 p.m. UTC | #5
On 23-02-08 19:10:17, Abel Vesa wrote:
> On 23-02-08 17:40:03, Johan Hovold wrote:
> > On Mon, Feb 06, 2023 at 05:11:01PM +0200, Abel Vesa wrote:
> > > On 23-02-03 10:49:24, Johan Hovold wrote:
> > > > On Fri, Feb 03, 2023 at 10:18:05AM +0200, Abel Vesa wrote:
> > > > > Add compatible for both PCIe found on SM8550.
> > > > > Also add the cnoc_pcie_sf_axi clock needed by the SM8550.
> > > > 
> > > > nit: You're now also adding 'noc_aggr'
> > > > 
> > > > > Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> > > > > Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> > > > > Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
> > > > > ---
> > 
> > > > > @@ -182,10 +182,10 @@ 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[12];
> > > > > +	struct clk_bulk_data clks[14];
> > > > >  	int num_clks;
> > > > >  	struct regulator_bulk_data supplies[2];
> > > > > -	struct reset_control *pci_reset;
> > > > > +	struct reset_control *rst;
> > > > 
> > > > Please name this one 'reset' or 'resets' (e.g. to avoid hard to parse
> > > > things like res->rst below).
> > > 
> > > Well, it would then be inconsitent with 2_3_3 and 2_9_0, which both use
> > > rst.
> > 
> > Yeah, I saw that. Fortunately these resources are completely
> > independent, but whatever.
> 
> Will do it in the next version then.
> 
> >  
> > > > >  };
> > > > >  
> > > > >  struct qcom_pcie_resources_2_9_0 {
> > > > > @@ -1177,9 +1177,9 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie)
> > > > >  	unsigned int idx;
> > > > >  	int ret;
> > > > >  
> > > > > -	res->pci_reset = devm_reset_control_get_exclusive(dev, "pci");
> > > > > -	if (IS_ERR(res->pci_reset))
> > > > > -		return PTR_ERR(res->pci_reset);
> > > > > +	res->rst = devm_reset_control_array_get_exclusive(dev);
> > > > > +	if (IS_ERR(res->rst))
> > > > > +		return PTR_ERR(res->rst);
> > > > 
> > > > So the reset array implementation apparently both asserts and deasserts
> > > > the resets in the order specified in DT (i.e. does not deassert in
> > > > reverse order).
> > > > 
> > > > Is that ok also for the new "pci" and "link_down" resets?
> > > 
> > > According to the HPG, yes, this is perfectly fine. It specifically says
> > > to assert the pcie reset and then continues saying to assert the
> > > link_down reset.
> > 
> > Ok, but that doesn't really say anything about whether it's ok to
> > *deassert* them in the same order, which was what I asked about.
> 
> Actually, what I wanted to say is that the HPG says something like this:
> 
> "assert pcie reset, then assert link_down"
> 
> and then at the end it literaly repeats the same phrase.

but uses deassert instead of assert ...

> 
> 
> 
> 
> 
> > 
> > Johan
Johan Hovold Feb. 8, 2023, 5:14 p.m. UTC | #6
On Wed, Feb 08, 2023 at 07:11:08PM +0200, Abel Vesa wrote:
> On 23-02-08 19:10:17, Abel Vesa wrote:
> > On 23-02-08 17:40:03, Johan Hovold wrote:
> > > On Mon, Feb 06, 2023 at 05:11:01PM +0200, Abel Vesa wrote:
> > > > On 23-02-03 10:49:24, Johan Hovold wrote:
> > > > > On Fri, Feb 03, 2023 at 10:18:05AM +0200, Abel Vesa wrote:
> > > > > > Add compatible for both PCIe found on SM8550.
> > > > > > Also add the cnoc_pcie_sf_axi clock needed by the SM8550.
> > > > > 
> > > > > nit: You're now also adding 'noc_aggr'
> > > > > 
> > > > > > Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> > > > > > Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> > > > > > Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
> > > > > > ---
> > > 
> > > > > > @@ -182,10 +182,10 @@ 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[12];
> > > > > > +	struct clk_bulk_data clks[14];
> > > > > >  	int num_clks;
> > > > > >  	struct regulator_bulk_data supplies[2];
> > > > > > -	struct reset_control *pci_reset;
> > > > > > +	struct reset_control *rst;
> > > > > 
> > > > > Please name this one 'reset' or 'resets' (e.g. to avoid hard to parse
> > > > > things like res->rst below).
> > > > 
> > > > Well, it would then be inconsitent with 2_3_3 and 2_9_0, which both use
> > > > rst.
> > > 
> > > Yeah, I saw that. Fortunately these resources are completely
> > > independent, but whatever.
> > 
> > Will do it in the next version then.

Or just leave it as is.

> > > > > >  };
> > > > > >  
> > > > > >  struct qcom_pcie_resources_2_9_0 {
> > > > > > @@ -1177,9 +1177,9 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie)
> > > > > >  	unsigned int idx;
> > > > > >  	int ret;
> > > > > >  
> > > > > > -	res->pci_reset = devm_reset_control_get_exclusive(dev, "pci");
> > > > > > -	if (IS_ERR(res->pci_reset))
> > > > > > -		return PTR_ERR(res->pci_reset);
> > > > > > +	res->rst = devm_reset_control_array_get_exclusive(dev);
> > > > > > +	if (IS_ERR(res->rst))
> > > > > > +		return PTR_ERR(res->rst);
> > > > > 
> > > > > So the reset array implementation apparently both asserts and deasserts
> > > > > the resets in the order specified in DT (i.e. does not deassert in
> > > > > reverse order).
> > > > > 
> > > > > Is that ok also for the new "pci" and "link_down" resets?
> > > > 
> > > > According to the HPG, yes, this is perfectly fine. It specifically says
> > > > to assert the pcie reset and then continues saying to assert the
> > > > link_down reset.
> > > 
> > > Ok, but that doesn't really say anything about whether it's ok to
> > > *deassert* them in the same order, which was what I asked about.
> > 
> > Actually, what I wanted to say is that the HPG says something like this:
> > 
> > "assert pcie reset, then assert link_down"
> > 
> > and then at the end it literaly repeats the same phrase.
> 
> but uses deassert instead of assert ...

Ok, then it seems to match the implementation. Thanks for clarifying.

Johan
diff mbox series

Patch

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index a232b04af048..6a70c9c6f98d 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -182,10 +182,10 @@  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[12];
+	struct clk_bulk_data clks[14];
 	int num_clks;
 	struct regulator_bulk_data supplies[2];
-	struct reset_control *pci_reset;
+	struct reset_control *rst;
 };
 
 struct qcom_pcie_resources_2_9_0 {
@@ -1177,9 +1177,9 @@  static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie)
 	unsigned int idx;
 	int ret;
 
-	res->pci_reset = devm_reset_control_get_exclusive(dev, "pci");
-	if (IS_ERR(res->pci_reset))
-		return PTR_ERR(res->pci_reset);
+	res->rst = devm_reset_control_array_get_exclusive(dev);
+	if (IS_ERR(res->rst))
+		return PTR_ERR(res->rst);
 
 	res->supplies[0].supply = "vdda";
 	res->supplies[1].supply = "vddpe-3v3";
@@ -1205,9 +1205,11 @@  static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie)
 	res->clks[idx++].id = "ddrss_sf_tbu";
 	res->clks[idx++].id = "aggre0";
 	res->clks[idx++].id = "aggre1";
+	res->clks[idx++].id = "noc_aggr";
 	res->clks[idx++].id = "noc_aggr_4";
 	res->clks[idx++].id = "noc_aggr_south_sf";
 	res->clks[idx++].id = "cnoc_qx";
+	res->clks[idx++].id = "cnoc_sf_axi";
 
 	num_opt_clks = idx - num_clks;
 	res->num_clks = idx;
@@ -1237,17 +1239,17 @@  static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
 	if (ret < 0)
 		goto err_disable_regulators;
 
-	ret = reset_control_assert(res->pci_reset);
-	if (ret < 0) {
-		dev_err(dev, "cannot assert pci reset\n");
+	ret = reset_control_assert(res->rst);
+	if (ret) {
+		dev_err(dev, "reset assert failed (%d)\n", ret);
 		goto err_disable_clocks;
 	}
 
 	usleep_range(1000, 1500);
 
-	ret = reset_control_deassert(res->pci_reset);
-	if (ret < 0) {
-		dev_err(dev, "cannot deassert pci reset\n");
+	ret = reset_control_deassert(res->rst);
+	if (ret) {
+		dev_err(dev, "reset deassert failed (%d)\n", ret);
 		goto err_disable_clocks;
 	}
 
@@ -1841,6 +1843,7 @@  static const struct of_device_id qcom_pcie_match[] = {
 	{ .compatible = "qcom,pcie-sm8350", .data = &cfg_1_9_0 },
 	{ .compatible = "qcom,pcie-sm8450-pcie0", .data = &cfg_1_9_0 },
 	{ .compatible = "qcom,pcie-sm8450-pcie1", .data = &cfg_1_9_0 },
+	{ .compatible = "qcom,pcie-sm8550", .data = &cfg_1_9_0 },
 	{ }
 };