Re: [PATCH v8 10/11] PCI: qcom: Add SM8550 PCIe support

[Johan Hovold]

On Mon, Feb 06, 2023 at 11:26:18PM +0200, Abel Vesa wrote:
> Add compatible for both PCIe found on SM8550.
> Also add the noc_aggr and cnoc_sf_axi clocks needed by the SM8550.
> 
> Signed-off-by: Abel Vesa <abel.vesa@xxxxxxxxxx>
> Reviewed-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxx>
> Reviewed-by: Manivannan Sadhasivam <mani@xxxxxxxxxx>
> ---

> @@ -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;
>  	}

I'd still like to know if it's indeed ok the deassert the 'pci' reset
before the new 'link_down' reset here which is what this code currently
would do (i.e. same order as for assert, while if you had used the bulk
reset API it would have deasserted in the reverse order).

Other than that:

Reviewed-by: Johan Hovold <johan+linaro@xxxxxxxxxx>