Re: [PATCH 2/2] phy: qcom: qmp-combo: check qmp_combo_com_init() in the typec callbacks

[Manivannan Sadhasivam <mani@kernel.org>]

On Wed, Sep 09, 2026 at 05:14:43PM +0200, Oleg Keri wrote:
> qmp_combo_typec_switch_set() and qmp_combo_typec_mux_set() tear the common
> block down and bring it straight back up on an orientation or altmode
> change, but they discard the result of the bringup:
> 
> 	qmp_combo_com_exit(qmp, true);
> 
> 	qmp_combo_com_init(qmp, true);
> 	if (qmp->usb_init_count)
> 		qmp_combo_usb_power_on(qmp->usb_phy);
> 
> If qmp_combo_com_init() fails - a regulator, a reset or
> clk_bulk_prepare_enable() - it unwinds what it had brought up and returns
> an error, leaving the clocks disabled.  Both callbacks then carry on and
> qmp_combo_usb_power_on(), and dp_aux_init() after it, write PHY registers
> with no clock running.
> 
> Propagate the failure instead.
> 
> Fixes: 2851117f8f42 ("phy: qcom-qmp-combo: Introduce orientation switching")
> Signed-off-by: Oleg Keri <okerixx@gmail.com>
> ---
>  drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> index 7d740ed0ce16..44b6f29926dc 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> @@ -4924,6 +4924,7 @@ static int qmp_combo_typec_switch_set(struct typec_switch_dev *sw,
>  {
>  	struct qmp_combo *qmp = typec_switch_get_drvdata(sw);
>  	const struct qmp_phy_cfg *cfg = qmp->cfg;
> +	int ret = 0;
>  
>  	if (orientation == qmp->orientation || orientation == TYPEC_ORIENTATION_NONE)
>  		return 0;
> @@ -4938,15 +4939,19 @@ static int qmp_combo_typec_switch_set(struct typec_switch_dev *sw,
>  			qmp_combo_usb_power_off(qmp->usb_phy);
>  		qmp_combo_com_exit(qmp, true);
>  
> -		qmp_combo_com_init(qmp, true);
> +		ret = qmp_combo_com_init(qmp, true);
> +		if (ret)
> +			goto out;
> +
>  		if (qmp->usb_init_count)
>  			qmp_combo_usb_power_on(qmp->usb_phy);
>  		if (qmp->dp_init_count)
>  			cfg->dp_aux_init(qmp);
>  	}
> +out:
>  	mutex_unlock(&qmp->phy_mutex);
>  
> -	return 0;
> +	return ret;

Please don't use this pattern. Success should always do 'return 0' directly. Do
'return ret' only in the error path.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy