From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752677AbcFNRmV (ORCPT ); Tue, 14 Jun 2016 13:42:21 -0400 Received: from hqemgate16.nvidia.com ([216.228.121.65]:12521 "EHLO hqemgate16.nvidia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751345AbcFNRmT (ORCPT ); Tue, 14 Jun 2016 13:42:19 -0400 X-PGP-Universal: processed; by hqnvupgp08.nvidia.com on Tue, 14 Jun 2016 10:39:39 -0700 Subject: Re: [PATCH v4] power_supply: power_supply_read_temp only if use_cnt > 0 To: Krzysztof Kozlowski , Sebastian Reichel , Dmitry Eremin-Solenikov , "David Woodhouse" References: <1465507719-819-1-git-send-email-rklein@nvidia.com> <575A8BBC.7040306@samsung.com> CC: , , Jon Hunter , From: Rhyland Klein Message-ID: <3cf2d32a-bd1b-03f1-2486-8311883639b2@nvidia.com> Date: Tue, 14 Jun 2016 13:42:09 -0400 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 MIME-Version: 1.0 In-Reply-To: <575A8BBC.7040306@samsung.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 6/10/2016 5:43 AM, Krzysztof Kozlowski wrote: > On 06/09/2016 11:28 PM, Rhyland Klein wrote: >> Change power_supply_read_temp() to use power_supply_get_property() >> so that it will check the use_cnt and ensure it is > 0. The use_cnt >> will be incremented at the end of __power_supply_register, so this >> will block to case where get_property can be called before the supply >> is fully registered. This fixes the issue show in the stack below: >> >> [ 1.452598] power_supply_read_temp+0x78/0x80 >> [ 1.458680] thermal_zone_get_temp+0x5c/0x11c >> [ 1.464765] thermal_zone_device_update+0x34/0xb4 >> [ 1.471195] thermal_zone_device_register+0x87c/0x8cc >> [ 1.477974] __power_supply_register+0x364/0x424 >> [ 1.484317] power_supply_register_no_ws+0x10/0x18 >> [ 1.490833] bq27xxx_battery_setup+0x10c/0x164 >> [ 1.497003] bq27xxx_battery_i2c_probe+0xd0/0x1b0 >> [ 1.503435] i2c_device_probe+0x174/0x240 >> [ 1.509172] driver_probe_device+0x1fc/0x29c >> [ 1.515167] __driver_attach+0xa4/0xa8 >> [ 1.520643] bus_for_each_dev+0x58/0x98 >> [ 1.526204] driver_attach+0x20/0x28 >> [ 1.531505] bus_add_driver+0x1c8/0x22c >> [ 1.537067] driver_register+0x68/0x108 >> [ 1.542630] i2c_register_driver+0x38/0x7c >> [ 1.548457] bq27xxx_battery_i2c_driver_init+0x18/0x20 >> [ 1.555321] do_one_initcall+0x38/0x12c >> [ 1.560886] kernel_init_freeable+0x148/0x1ec >> [ 1.566972] kernel_init+0x10/0xfc >> [ 1.572101] ret_from_fork+0x10/0x40 >> >> Also make the same change to ps_get_max_charge_cntl_limit() and >> ps_get_cur_chrage_cntl_limit() to be safe. Lastly, change the return >> value of power_supply_get_property() to -EAGAIN from -ENODEV if >> use_cnt <= 0. >> >> Fixes: 297d716f6260 ("power_supply: Change ownership from driver to core") >> Cc: stable@vger.kernel.org >> Signed-off-by: Rhyland Klein >> --- >> v4: >> - Fixed return check conditions >> - Removed change to power_supply_get_property return code >> - removed whitespace change >> >> v3: >> - Changed calls to ->get_property() to use common >> power_supply_get_property() >> - reworded description, added "Fixes" line >> - Changed return value of power_supply_get_property() to -EAGAIN >> >> v2: >> - Added cc stable >> - changed return to -EAGAIN in case of use_cnt < 1 >> - Removed WARNING >> - return value check added in additional patch: >> https://patchwork.kernel.org/patch/9158805/ >> >> drivers/power/power_supply_core.c | 27 ++++++++++++++++----------- >> 1 file changed, 16 insertions(+), 11 deletions(-) > > Looks okay: > > Reviewed-by: Krzysztof Kozlowski > > Best regards, > Krzysztof > > I sent another patch which is a follow up to this one, which changes the return value of power_supply_get_property() to return -EAGAIN if it is called during __power_supply_register(). Turns out the thermal framework (at least) will treat EAGAIN differently and since we were on the fence about what to return, it made sense to go ahead and do that to make everyone happy. https://lkml.org/lkml/2016/6/13/943 -rhyland -- nvpublic