[PATCH v2 2/4] phy: qcom: qmp-combo: track whether the cached typec_mux mode was committed to hardware

[Bryan O'Donoghue]

On 21/05/2026 02:09, Michael Scott wrote:
> qmp_combo_typec_mux_set() updates qmp->qmpphy_mode (the cached state)
> unconditionally, but only reprograms hardware when qmp->init_count is
> non-zero. If pmic_glink_altmode (or any other typec_mux consumer)
> calls into the PHY before DWC3 has performed phy_init() -- a real
> ordering observed during testing of USB-C role-switch enablement on
> Snapdragon X (X1E80100) -- the cache transitions away from the
> probe default QMPPHY_MODE_USB3DP but the hardware is never touched.
> 
> Subsequent calls (for example on partner detach, where TYPEC_STATE_SAFE
> also resolves to QMPPHY_MODE_USB3_ONLY in the !DP-SVID branch) then
> match the cached mode and the function bails out early with:
> 
>    qcom-qmp-combo-phy faXX000.phy: typec_mux_set: same qmpphy mode, bail out
> 
> leaving the lane mux in whatever configuration it powered up in. On
> the Dell Latitude 7455 this manifests as the SS lanes being left in
> the default state when the first altmode notification arrives during
> DWC3 probe, with the function bailing out on every subsequent attach.
> 
> Track separately whether the cached mode has actually been committed
> to hardware. The bail-out optimization is only safe when the cache
> truly reflects the hardware:
> 
>    - qmp_combo_typec_mux_set(): bail only when the cached mode matches
>      and was committed; clear the committed flag whenever the cache is
>      updated, set it again after a successful reprogram inside the
>      init_count-guarded block.
> 
>    - qmp_combo_com_init(): set the committed flag at the end of a
>      successful init, since com_init() programs registers from the
>      cached qmpphy_mode.
> 
> No behavioural change on platforms where typec_mux_set never fires
> before phy_init -- committed remains true through normal operation.
> 
> Signed-off-by: Michael Scott <mike.scott at oss.qualcomm.com>
> ---
>   drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 25 +++++++++++++++++++++--
>   1 file changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> index 0db200292642..e28bc1cc7a78 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> @@ -2295,6 +2295,7 @@ struct qmp_combo {
>   	struct mutex phy_mutex;
>   	int init_count;
>   	enum qmpphy_mode qmpphy_mode;
> +	bool qmpphy_mode_committed;
>   
>   	struct phy *usb_phy;
>   	enum phy_mode phy_mode;
> @@ -3754,6 +3755,9 @@ static int qmp_combo_com_init(struct qmp_combo *qmp, bool force)
>   	qphy_setbits(qmp->pcs, cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL],
>   			SW_PWRDN);
>   
> +	/* com_init() just programmed registers from qmp->qmpphy_mode. */
> +	qmp->qmpphy_mode_committed = true;
> +
>   	return 0;
>   
>   err_disable_clocks:
> @@ -4509,9 +4513,22 @@ static int qmp_combo_typec_mux_set(struct typec_mux_dev *mux, struct typec_mux_s
>   		new_mode = QMPPHY_MODE_USB3_ONLY;
>   	}
>   
> +	/*
> +	 * Fast-path bail only when the cached mode is also known to be
> +	 * committed to hardware. The cache may be ahead of the hardware
> +	 * if a typec_mux_set arrived while the PHY had not yet been
> +	 * initialised (init_count == 0); in that case the cache update
> +	 * below was the only thing that ran, and we still need to drive
> +	 * the registers when the PHY does come up.
> +	 */
>   	if (new_mode == qmp->qmpphy_mode) {
> -		dev_dbg(qmp->dev, "typec_mux_set: same qmpphy mode, bail out\n");
> -		return 0;
> +		if (qmp->qmpphy_mode_committed) {
> +			dev_dbg(qmp->dev,
> +				"typec_mux_set: same qmpphy mode (committed), bail out\n");
> +			return 0;
> +		}
> +		dev_dbg(qmp->dev,
> +			"typec_mux_set: same qmpphy mode but uncommitted; reprogramming\n");
>   	}
>   
>   	if (qmp->qmpphy_mode != QMPPHY_MODE_USB3_ONLY && qmp->dp_powered_on) {
> @@ -4523,6 +4540,7 @@ static int qmp_combo_typec_mux_set(struct typec_mux_dev *mux, struct typec_mux_s
>   		qmp->qmpphy_mode, new_mode);
>   
>   	qmp->qmpphy_mode = new_mode;
> +	qmp->qmpphy_mode_committed = false;
>   
>   	if (qmp->init_count) {
>   		if (qmp->usb_init_count)
> @@ -4551,6 +4569,9 @@ static int qmp_combo_typec_mux_set(struct typec_mux_dev *mux, struct typec_mux_s
>   			if (qmp->dp_init_count)
>   				cfg->dp_aux_init(qmp);
>   		}
> +
> +		/* Reprogram complete; cache now reflects hardware. */
> +		qmp->qmpphy_mode_committed = true;
>   	}
>   
>   	return 0;

Can we not make the commit to hardware atomic from the perspective of 
the caller ?

i.e. use a workqueue and a completion timeout when setting ?

---
bod