Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Sun, 7 Nov 2021 21:49:08 +0300
From:   Yauhen Kharuzhy <jekhor@gmail.com>
To:     Hans de Goede <hdegoede@redhat.com>
Cc:     Mark Gross <markgross@kernel.org>,
        Andy Shevchenko <andy@infradead.org>,
        Wolfram Sang <wsa@the-dreams.de>,
        Sebastian Reichel <sre@kernel.org>,
        MyungJoo Ham <myungjoo.ham@samsung.com>,
        Chanwoo Choi <cw00.choi@samsung.com>,
        Ard Biesheuvel <ardb@kernel.org>,
        Tsuchiya Yuto <kitakar@gmail.com>,
        platform-driver-x86@vger.kernel.org, linux-i2c@vger.kernel.org,
        linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-efi@vger.kernel.org
Subject: Re: [PATCH 09/13] power: supply: bq25890: Add bq25890_set_otg_cfg()
 helper
Message-ID: <YYgfpJCUDs+dU++a@jeknote.loshitsa1.net>
References: <20211030182813.116672-1-hdegoede@redhat.com>
 <20211030182813.116672-10-hdegoede@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20211030182813.116672-10-hdegoede@redhat.com>
Precedence: bulk

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