Re: [PATCH v6 1/2] phy: qcom: pcie: Determine has_nocsr_reset dynamically

[Aleksandrs Vinarskis]

On 3/19/25 10:45, Wenbin Yao wrote:
> From: Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxxxx>
>
> Decide the in-driver logic based on whether the nocsr reset is present
> and defer checking the appropriateness of that to dt-bindings to save
> on boilerplate.
>
> Reset controller APIs are fine consuming a nullptr, so no additional
> checks are necessary there.
>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxxxx>
> Signed-off-by: Wenbin Yao <quic_wenbyao@xxxxxxxxxxx>
> Reviewed-by: Abel Vesa <abel.vesa@xxxxxxxxxx>
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>
> Reviewed-by: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>


Successfully tested on Snapdragon X1-26-100 on Asus Zenbook A14.

This fixes pcie6a_phy with "qcom,x1p42100-qmp-gen4x4-pcie-phy" as 
compatible, which was not working before.

Tested-by: Aleksandrs Vinarskis <alex.vinarskis@xxxxxxxxx>

> ---
>   drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 17 ++++-------------
>   1 file changed, 4 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> index 018bbb300830..38dbe690f2d5 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> @@ -2969,8 +2969,6 @@ struct qmp_phy_cfg {
>   
>   	bool skip_start_delay;
>   
> -	bool has_nocsr_reset;
> -
>   	/* QMP PHY pipe clock interface rate */
>   	unsigned long pipe_clock_rate;
>   
> @@ -3934,7 +3932,6 @@ static const struct qmp_phy_cfg sm8550_qmp_gen4x2_pciephy_cfg = {
>   
>   	.pwrdn_ctrl		= SW_PWRDN | REFCLK_DRV_DSBL,
>   	.phy_status		= PHYSTATUS_4_20,
> -	.has_nocsr_reset	= true,
>   
>   	/* 20MHz PHY AUX Clock */
>   	.aux_clock_rate		= 20000000,
> @@ -3967,7 +3964,6 @@ static const struct qmp_phy_cfg sm8650_qmp_gen4x2_pciephy_cfg = {
>   
>   	.pwrdn_ctrl		= SW_PWRDN | REFCLK_DRV_DSBL,
>   	.phy_status		= PHYSTATUS_4_20,
> -	.has_nocsr_reset	= true,
>   
>   	/* 20MHz PHY AUX Clock */
>   	.aux_clock_rate		= 20000000,
> @@ -4087,7 +4083,6 @@ static const struct qmp_phy_cfg x1e80100_qmp_gen4x2_pciephy_cfg = {
>   
>   	.pwrdn_ctrl		= SW_PWRDN | REFCLK_DRV_DSBL,
>   	.phy_status		= PHYSTATUS_4_20,
> -	.has_nocsr_reset	= true,
>   };
>   
>   static const struct qmp_phy_cfg x1e80100_qmp_gen4x4_pciephy_cfg = {
> @@ -4121,7 +4116,6 @@ static const struct qmp_phy_cfg x1e80100_qmp_gen4x4_pciephy_cfg = {
>   
>   	.pwrdn_ctrl		= SW_PWRDN | REFCLK_DRV_DSBL,
>   	.phy_status		= PHYSTATUS_4_20,
> -	.has_nocsr_reset	= true,
>   };
>   
>   static const struct qmp_phy_cfg x1e80100_qmp_gen4x8_pciephy_cfg = {
> @@ -4153,7 +4147,6 @@ static const struct qmp_phy_cfg x1e80100_qmp_gen4x8_pciephy_cfg = {
>   
>   	.pwrdn_ctrl		= SW_PWRDN | REFCLK_DRV_DSBL,
>   	.phy_status		= PHYSTATUS_4_20,
> -	.has_nocsr_reset	= true,
>   };
>   
>   static void qmp_pcie_init_port_b(struct qmp_pcie *qmp, const struct qmp_phy_cfg_tbls *tbls)
> @@ -4456,12 +4449,10 @@ static int qmp_pcie_reset_init(struct qmp_pcie *qmp)
>   	if (ret)
>   		return dev_err_probe(dev, ret, "failed to get resets\n");
>   
> -	if (cfg->has_nocsr_reset) {
> -		qmp->nocsr_reset = devm_reset_control_get_exclusive(dev, "phy_nocsr");
> -		if (IS_ERR(qmp->nocsr_reset))
> -			return dev_err_probe(dev, PTR_ERR(qmp->nocsr_reset),
> -						"failed to get no-csr reset\n");
> -	}
> +	qmp->nocsr_reset = devm_reset_control_get_optional_exclusive(dev, "phy_nocsr");
> +	if (IS_ERR(qmp->nocsr_reset))
> +		return dev_err_probe(dev, PTR_ERR(qmp->nocsr_reset),
> +				     "failed to get no-csr reset\n");
>   
>   	return 0;
>   }