Re: [PATCH 2/3] phy: sun4i-usb: Refactor VBUS detection to match Linux

[Andre Przywara <andre.przywara@arm.com>]

On Sun, 12 Sep 2021 09:22:41 -0500
Samuel Holland <samuel@sholland.org> wrote:

Hi,

> The Linux driver checks the VBUS detection GPIO first; then VBUS power
> supply; then finally assumes VBUS is present. When adding VBUS power
> supply support, we want to match that order, so we get the same behavior
> in case both a GPIO and a power supply are provided in the device tree.
> 
> So refactor the function a bit to remove the early return, and use the
> same "assume VBUS is present" final fallback.
> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>

Let's see if testing reveals any subtle issues ;-)

Acked-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre

> ---
> 
>  drivers/phy/allwinner/phy-sun4i-usb.c | 25 ++++++++++++-------------
>  1 file changed, 12 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/phy/allwinner/phy-sun4i-usb.c b/drivers/phy/allwinner/phy-sun4i-usb.c
> index 5302b809ee6..827ecd70f27 100644
> --- a/drivers/phy/allwinner/phy-sun4i-usb.c
> +++ b/drivers/phy/allwinner/phy-sun4i-usb.c
> @@ -391,20 +391,19 @@ int sun4i_usb_phy_vbus_detect(struct phy *phy)
>  {
>  	struct sun4i_usb_phy_data *data = dev_get_priv(phy->dev);
>  	struct sun4i_usb_phy_plat *usb_phy = &data->usb_phy[phy->id];
> -	int err, retries = 3;
> -
> -	if (usb_phy->gpio_vbus_det < 0)
> -		return usb_phy->gpio_vbus_det;
> -
> -	err = gpio_get_value(usb_phy->gpio_vbus_det);
> -	/*
> -	 * Vbus may have been provided by the board and just been turned of
> -	 * some milliseconds ago on reset, what we're measuring then is a
> -	 * residual charge on Vbus, sleep a bit and try again.
> -	 */
> -	while (err > 0 && retries--) {
> -		mdelay(100);
> +	int err = 1, retries = 3;
> +
> +	if (usb_phy->gpio_vbus_det >= 0) {
>  		err = gpio_get_value(usb_phy->gpio_vbus_det);
> +		/*
> +		 * Vbus may have been provided by the board and just turned off
> +		 * some milliseconds ago on reset. What we're measuring then is
> +		 * a residual charge on Vbus. Sleep a bit and try again.
> +		 */
> +		while (err > 0 && retries--) {
> +			mdelay(100);
> +			err = gpio_get_value(usb_phy->gpio_vbus_det);
> +		}
>  	}
>  
>  	return err;