Re: [PATCH] usb: typec: hd3ss3220: track VBUS enable state per consumer

[Jan Remmet]

Am 19.08.26 um 17:20 schrieb Chang Wu:
> regulator_is_enabled() reports the aggregate regulator state, not
> whether this consumer holds an enable reference. If another consumer
> enables VBUS first, the driver can skip its own regulator_enable() call
> and later attempt to drop a reference it never acquired, triggering an
> unbalanced regulator disable warning.
> 
> Track successful enable and disable calls locally. Keep the state
> unchanged when an operation fails so a later role or ID notification
> retries the operation while this consumer keeps balanced references.
> 
> Fixes: b3f9d6e491fd ("usb: typec: hd3ss3220: Check if regulator needs to be switched")
> Cc: stable@xxxxxxxxxxxxxxx
> Link: https://github.com/qualcomm-linux/kernel/issues/472
> Signed-off-by: Chang Wu <kunjinkao.jp@xxxxxxxxx>
tested on our i.MX95 devboard with switching device / host scenarios.

Tested-by: Jan Remmet <j.remmet@xxxxxxxxx>
> ---
> Testing:
> - scripts/checkpatch.pl --strict: passed
> - Qualcomm CI checkpatch, sparse, DT and UAPI checks: passed
> - Not tested on hardware
> 
>   drivers/usb/typec/hd3ss3220.c | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/typec/hd3ss3220.c b/drivers/usb/typec/hd3ss3220.c
> index d0de5a2488f9..4eec90c82bae 100644
> --- a/drivers/usb/typec/hd3ss3220.c
> +++ b/drivers/usb/typec/hd3ss3220.c
> @@ -62,6 +62,7 @@ struct hd3ss3220 {
>   	int id_irq;
>   
>   	struct regulator *vbus;
> +	bool vbus_enabled;
>   };
>   
>   static int hd3ss3220_set_power_opmode(struct hd3ss3220 *hd3ss3220, int power_opmode)
> @@ -208,7 +209,7 @@ static void hd3ss3220_regulator_control(struct hd3ss3220 *hd3ss3220, bool on)
>   {
>   	int ret;
>   
> -	if (regulator_is_enabled(hd3ss3220->vbus) == on)
> +	if (hd3ss3220->vbus_enabled == on)
>   		return;
>   
>   	if (on)
> @@ -216,9 +217,13 @@ static void hd3ss3220_regulator_control(struct hd3ss3220 *hd3ss3220, bool on)
>   	else
>   		ret = regulator_disable(hd3ss3220->vbus);
>   
> -	if (ret)
> +	if (ret) {
>   		dev_err(hd3ss3220->dev,
>   			"vbus regulator %s failed: %d\n", on ? "enable" : "disable", ret);
> +		return;
> +	}
> +
> +	hd3ss3220->vbus_enabled = on;
>   }
>   
>   static void hd3ss3220_set_role(struct hd3ss3220 *hd3ss3220)
> 
> base-commit: e1e6e541c5c9cf548e9fdc35fc26808c82074440