Re: [RFC PATCH 2/5] gnss: ubx: use new helper to remove open coded regulator handling

[Johan Hovold]

On Tue, May 23, 2023 at 08:43:07AM +0200, Wolfram Sang wrote:
> v_bckp shall always be enabled as long as the device exists. We now have
> a regulator helper for that, use it.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>
> ---
>  drivers/gnss/ubx.c | 26 ++++----------------------
>  1 file changed, 4 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gnss/ubx.c b/drivers/gnss/ubx.c
> index c01e1e875727..c8d027973b85 100644
> --- a/drivers/gnss/ubx.c
> +++ b/drivers/gnss/ubx.c
> @@ -17,7 +17,6 @@
>  #include "serial.h"
>  
>  struct ubx_data {
> -	struct regulator *v_bckp;
>  	struct regulator *vcc;
>  };
>  
> @@ -106,30 +105,16 @@ static int ubx_probe(struct serdev_device *serdev)
>  		goto err_free_gserial;
>  	}
>  
> -	data->v_bckp = devm_regulator_get_optional(&serdev->dev, info->v_bckp_name);
> -	if (IS_ERR(data->v_bckp)) {
> -		ret = PTR_ERR(data->v_bckp);
> -		if (ret == -ENODEV)
> -			data->v_bckp = NULL;
> -		else
> -			goto err_free_gserial;
> -	}
> -
> -	if (data->v_bckp) {
> -		ret = regulator_enable(data->v_bckp);
> -		if (ret)
> -			goto err_free_gserial;
> -	}
> +	ret = devm_regulator_get_enable_optional(&serdev->dev, info->v_bckp_name);
> +	if (ret < 0 && ret != -ENODEV)
> +		goto err_free_gserial;

I'm a bit torn about this one as I'm generally sceptical of devres and
especially helpers that enable or register resources, which just tends to
lead to subtle bugs.

But if there's one place where this new helper, which importantly does
not return a pointer to the regulator, may be useful I guess it's here.

Care to respin this one after dropping the merge patch?

Johan