From patchwork Thu May 26 10:35:49 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jon Hunter X-Patchwork-Id: 626635 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3rFlt85nlcz9t3w for ; Thu, 26 May 2016 20:36:00 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753262AbcEZKf7 (ORCPT ); Thu, 26 May 2016 06:35:59 -0400 Received: from hqemgate14.nvidia.com ([216.228.121.143]:15764 "EHLO hqemgate14.nvidia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753506AbcEZKf6 (ORCPT ); Thu, 26 May 2016 06:35:58 -0400 Received: from hqnvupgp08.nvidia.com (Not Verified[216.228.121.13]) by hqemgate14.nvidia.com id ; Thu, 26 May 2016 03:35:58 -0700 Received: from HQHUB101.nvidia.com ([172.20.187.24]) by hqnvupgp08.nvidia.com (PGP Universal service); Thu, 26 May 2016 03:34:19 -0700 X-PGP-Universal: processed; by hqnvupgp08.nvidia.com on Thu, 26 May 2016 03:34:19 -0700 Received: from UKMAIL102.nvidia.com (10.26.138.15) by HQHUB101.nvidia.com (172.20.187.24) with Microsoft SMTP Server (TLS) id 8.3.406.0; Thu, 26 May 2016 03:35:56 -0700 Received: from [10.26.11.89] (10.26.11.89) by UKMAIL102.nvidia.com (10.26.138.15) with Microsoft SMTP Server (TLS) id 15.0.1130.7; Thu, 26 May 2016 10:35:51 +0000 Subject: Re: [PATCH] arm64: defconfig: Enable cros-ec and battery driver To: Rhyland Klein , Thierry Reding , Krzysztof Kozlowski , Sebastian Reichel , David Woodhouse , Dmitry Eremin-Solenikov References: <1462290318-9074-1-git-send-email-rklein@nvidia.com> <5744609A.1000008@nvidia.com> <324dfe74-4fc0-d500-91ac-2a802562e92f@nvidia.com> <5745853B.1040304@nvidia.com> <57458693.3050700@nvidia.com> <20160525154618.GD13765@ulmo.ba.sec> <9411ff33-e375-8286-8690-fe7fcac1c14b@nvidia.com> <5745CE75.7010603@nvidia.com> <5745D2DD.6080300@nvidia.com> <1c6df907-ea1f-201b-a36e-8311c5b2b3b1@nvidia.com> <5745E031.7010406@nvidia.com> <5c03a025-f31d-fa18-b973-0b026ede9c5c@nvidia.com> CC: Stephen Warren , Alexandre Courbot , , From: Jon Hunter Message-ID: <5746D185.30006@nvidia.com> Date: Thu, 26 May 2016 11:35:49 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0 MIME-Version: 1.0 In-Reply-To: <5c03a025-f31d-fa18-b973-0b026ede9c5c@nvidia.com> X-Originating-IP: [10.26.11.89] X-ClientProxiedBy: UKMAIL102.nvidia.com (10.26.138.15) To UKMAIL102.nvidia.com (10.26.138.15) Sender: linux-tegra-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-tegra@vger.kernel.org On 25/05/16 20:44, Rhyland Klein wrote: ... > And you might be completely correct, that is something that can only > happen specifically with the bq27xxx driver. In which case, making the > fix there should be the fix. I just know from the commit log (and some > previous work with power supply drivers) that the case of get_property > being called during registration has caused problems before. That's why > I am trying to make sure we cover the generic case if it exists. Using > scheduled work is common for power_supplies to regularly update their > status. Yes but only appears to be the bq27xxx and seems to be related to this issue. To be honest, if the maintainers are ok with your proposal then it is fine with me, but I was just thinking that this feels like a bq27xxx issue. > As for your proposed patches for bq27xxx, I think the latest one you > suggested (@12:36PM EST) with the change for > battery_update->battery_poll as well makes the most sense for bq27xxx. I > would like to point out though that if we patch this, the cache won't be > populated for the first TEMP request, which has the same end effect as > the patch I proposed to power_supply_read_temp. I believe both will > return 0 for the temp. Actually, I gave it a test and instead of returning zero it returns absolute zero (0K or -273C!). So that is probably no good. Something like Thierry was suggesting could work. Another thought I had was ... Cheers Jon diff --git a/drivers/power/bq27xxx_battery.c b/drivers/power/bq27xxx_battery.c index 45f6ebf88df6..77488183df6b 100644 --- a/drivers/power/bq27xxx_battery.c +++ b/drivers/power/bq27xxx_battery.c @@ -674,12 +674,15 @@ static int bq27xxx_battery_read_health(struct bq27xxx_device_info *di) return POWER_SUPPLY_HEALTH_GOOD; } -void bq27xxx_battery_update(struct bq27xxx_device_info *di) +static void __bq27xxx_battery_update(struct bq27xxx_device_info *di, + struct power_supply *psy) { struct bq27xxx_reg_cache cache = {0, }; bool has_ci_flag = di->chip == BQ27000 || di->chip == BQ27010; bool has_singe_flag = di->chip == BQ27000 || di->chip == BQ27010; + WARN_ON(!psy); + cache.flags = bq27xxx_read(di, BQ27XXX_REG_FLAGS, has_singe_flag); if ((cache.flags & 0xff) == 0xff) cache.flags = -1; /* read error */ @@ -717,13 +720,25 @@ void bq27xxx_battery_update(struct bq27xxx_device_info *di) di->charge_design_full = bq27xxx_battery_read_dcap(di); } - if (di->cache.capacity != cache.capacity) - power_supply_changed(di->bat); + if (psy && di->cache.capacity != cache.capacity) + power_supply_changed(psy); if (memcmp(&di->cache, &cache, sizeof(cache)) != 0) di->cache = cache; di->last_update = jiffies; + + if (poll_interval > 0) { + /* The timer does not have to be accurate. */ + set_timer_slack(&di->work.timer, poll_interval * HZ / 4); + schedule_delayed_work(&di->work, poll_interval * HZ); + } +} + +void bq27xxx_battery_update(struct bq27xxx_device_info *di) +{ + cancel_delayed_work_sync(&di->work); + __bq27xxx_battery_update(di, di->bat); } EXPORT_SYMBOL_GPL(bq27xxx_battery_update); @@ -733,13 +748,7 @@ static void bq27xxx_battery_poll(struct work_struct *work) container_of(work, struct bq27xxx_device_info, work.work); - bq27xxx_battery_update(di); - - if (poll_interval > 0) { - /* The timer does not have to be accurate. */ - set_timer_slack(&di->work.timer, poll_interval * HZ / 4); - schedule_delayed_work(&di->work, poll_interval * HZ); - } + __bq27xxx_battery_update(di, di->bat); } /* @@ -874,7 +883,7 @@ static int bq27xxx_battery_get_property(struct power_supply *psy, mutex_lock(&di->lock); if (time_is_before_jiffies(di->last_update + 5 * HZ)) { cancel_delayed_work_sync(&di->work); - bq27xxx_battery_poll(&di->work.work); + __bq27xxx_battery_update(di, psy); } mutex_unlock(&di->lock);