Re: [PATCH 09/13] power: supply: bq25890: Add bq25890_set_otg_cfg() helper

[Yauhen Kharuzhy <jekhor@gmail.com>]

On Sat, Oct 30, 2021 at 08:28:09PM +0200, Hans de Goede wrote:
> Add a bq25890_set_otg_cfg() helper function, this is a preparation
> patch for adding regulator support.

The same notice here: if we enabling the boost mode to supply USB OTG
device, we should disable charging and vice versa. I don't remember if
enabling of OTG with CHG enabled really caused issues but I avoided this
in my Yoga Book work.

I made quick check — seems that charging can be enabled during of boost
operation, there are no visible side effects in registers and no
limitation in the datasheet, so your approach may be good.

> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/power/supply/bq25890_charger.c | 28 ++++++++++++++------------
>  1 file changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
> index 163ca5d761aa..7504e30f1e4d 100644
> --- a/drivers/power/supply/bq25890_charger.c
> +++ b/drivers/power/supply/bq25890_charger.c
> @@ -773,6 +773,17 @@ static int bq25890_power_supply_init(struct bq25890_device *bq)
>  	return PTR_ERR_OR_ZERO(bq->charger);
>  }
>  
> +static int bq25890_set_otg_cfg(struct bq25890_device *bq, u8 val)
> +{
> +	int ret;
> +
> +	ret = bq25890_field_write(bq, F_OTG_CFG, val);
> +	if (ret < 0)
> +		dev_err(bq->dev, "Error switching to boost/charger mode: %d\n", ret);
> +
> +	return ret;
> +}
> +
>  static void bq25890_usb_work(struct work_struct *data)
>  {
>  	int ret;
> @@ -782,25 +793,16 @@ static void bq25890_usb_work(struct work_struct *data)
>  	switch (bq->usb_event) {
>  	case USB_EVENT_ID:
>  		/* Enable boost mode */
> -		ret = bq25890_field_write(bq, F_OTG_CFG, 1);
> -		if (ret < 0)
> -			goto error;
> +		bq25890_set_otg_cfg(bq, 1);
>  		break;
>  
>  	case USB_EVENT_NONE:
>  		/* Disable boost mode */
> -		ret = bq25890_field_write(bq, F_OTG_CFG, 0);
> -		if (ret < 0)
> -			goto error;
> -
> -		power_supply_changed(bq->charger);
> +		ret = bq25890_set_otg_cfg(bq, 0);
> +		if (ret == 0)
> +			power_supply_changed(bq->charger);
>  		break;
>  	}
> -
> -	return;
> -
> -error:
> -	dev_err(bq->dev, "Error switching to boost/charger mode.\n");
>  }
>  
>  static int bq25890_usb_notifier(struct notifier_block *nb, unsigned long val,
> -- 
> 2.31.1
> 

-- 
Yauhen Kharuzhy