From patchwork Wed Apr 12 18:18:07 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans de Goede X-Patchwork-Id: 9678139 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 42F0C60383 for ; Wed, 12 Apr 2017 18:18:17 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 39C542867F for ; Wed, 12 Apr 2017 18:18:17 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 2EB6C28694; Wed, 12 Apr 2017 18:18:17 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 7FB1A28695 for ; Wed, 12 Apr 2017 18:18:16 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752372AbdDLSSP (ORCPT ); Wed, 12 Apr 2017 14:18:15 -0400 Received: from mx1.redhat.com ([209.132.183.28]:51867 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752286AbdDLSSP (ORCPT ); Wed, 12 Apr 2017 14:18:15 -0400 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id E3AB4C04BD3E; Wed, 12 Apr 2017 18:18:14 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com E3AB4C04BD3E Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=hdegoede@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com E3AB4C04BD3E Received: from shalem.localdomain.com (ovpn-116-234.ams2.redhat.com [10.36.116.234]) by smtp.corp.redhat.com (Postfix) with ESMTP id 861F01715A; Wed, 12 Apr 2017 18:18:13 +0000 (UTC) From: Hans de Goede To: Sebastian Reichel Cc: Hans de Goede , Liam Breck , Tony Lindgren , linux-pm@vger.kernel.org, Liam Breck Subject: [PATCH v4 2/3] power: supply: bq24190_charger: Do not reset the charger on probe by default Date: Wed, 12 Apr 2017 20:18:07 +0200 Message-Id: <20170412181808.31768-3-hdegoede@redhat.com> In-Reply-To: <20170412181808.31768-1-hdegoede@redhat.com> References: <20170412181808.31768-1-hdegoede@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Wed, 12 Apr 2017 18:18:15 +0000 (UTC) Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Resetting the charger should never be necessary it should always have sane values programmed. If it is running with invalid values while we are not running (system turned off or suspended) there is a big problem as that may lead to overcharging the battery. The reset in suspend() is meant to put the charger back into default mode, but this is not necessary and not a good idea. If the charger has been programmed with a higher max charge_current / charge_voltage then putting it back in default-mode will reset those to the safe power-on defaults, leading to slower charging, or charging to a lower voltage (and thus not using the full capacity) while suspended which is undesirable. Reprogramming the max charge_current / charge_voltage after the reset will not help here as that will put the charger back in host mode and start the i2c watchdog if the host then does not do anything for 40s (iow if we're suspended for more then 40s) the watchdog expires resetting the device to default-mode, including resetting all the registers to there safe power-on defaults. So the only way to keep using custom charge settings while suspending is to keep the charger in its normal running state with the i2c watchdog disabled. This is fine as the charger will still automatically switch from constant current to constant voltage and stop charging when the battery is full. Besides never being necessary resetting the charger also causes problems on systems where the charge voltage limit is set higher then the reset value, if this is the case and the charger is reset while charging and the battery voltage is between the 2 voltages, then about half the time the charger gets confused and claims to be charging (REG08 contains 0x64) but in reality the charger has decoupled itself from VBUS (Q1 off) and is drawing 0A from VBUS, leaving the system running from the battery. For all the above reasons this commit disables reset on probe and on suspend / resume. If a specific setup really needs / wants to do a rest in these cases, this can be requested by setting a reset-on-probe device property on the device. Cc: Liam Breck Cc: Tony Lindgren Signed-off-by: Hans de Goede --- Changes in v2: -This is a new patch in v2 of this patch-set Changes in v3: -Disable the reset code by default, but leave it around guarded by a check for a reset-on-probe device-property Changes in v4: -Rebase on top of current power-supply/next --- drivers/power/supply/bq24190_charger.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/drivers/power/supply/bq24190_charger.c b/drivers/power/supply/bq24190_charger.c index bd9e5c3..ad429b7 100644 --- a/drivers/power/supply/bq24190_charger.c +++ b/drivers/power/supply/bq24190_charger.c @@ -1382,9 +1382,11 @@ static int bq24190_hw_init(struct bq24190_dev_info *bdi) return -ENODEV; } - ret = bq24190_register_reset(bdi); - if (ret < 0) - return ret; + if (device_property_read_bool(bdi->dev, "reset-on-probe")) { + ret = bq24190_register_reset(bdi); + if (ret < 0) + return ret; + } ret = bq24190_set_mode_host(bdi); if (ret < 0) @@ -1547,7 +1549,8 @@ static int bq24190_remove(struct i2c_client *client) pm_runtime_put_noidle(bdi->dev); } - bq24190_register_reset(bdi); + if (device_property_read_bool(bdi->dev, "reset-on-probe")) + bq24190_register_reset(bdi); bq24190_sysfs_remove_group(bdi); power_supply_unregister(bdi->battery); power_supply_unregister(bdi->charger); @@ -1600,7 +1603,8 @@ static __maybe_unused int bq24190_pm_suspend(struct device *dev) pm_runtime_put_noidle(bdi->dev); } - bq24190_register_reset(bdi); + if (device_property_read_bool(bdi->dev, "reset-on-probe")) + bq24190_register_reset(bdi); if (error >= 0) { pm_runtime_mark_last_busy(bdi->dev); @@ -1625,8 +1629,10 @@ static __maybe_unused int bq24190_pm_resume(struct device *dev) pm_runtime_put_noidle(bdi->dev); } - bq24190_register_reset(bdi); - bq24190_set_mode_host(bdi); + if (device_property_read_bool(bdi->dev, "reset-on-probe")) { + bq24190_register_reset(bdi); + bq24190_set_mode_host(bdi); + } bq24190_read(bdi, BQ24190_REG_SS, &bdi->ss_reg); if (error >= 0) {